From 656151783cb9aa165d9dc99e82d7855ed3943b11 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Tue, 24 Mar 2026 23:01:19 -0700 Subject: [PATCH 1/8] feat(cli): show credential auth status in tool info (#1572) * feat(cli): show credential auth status in `tool info` `ironclaw tool info` now checks the secrets store and shows whether each required credential is configured or missing, consolidated into a single Auth section that deduplicates across http.credentials, auth, and setup.required_secrets. Secrets already shown in Auth are filtered from the Secrets section to avoid redundancy. Co-Authored-By: Claude Opus 4.6 (1M context) * fix(cli): address review feedback on tool info auth status - Fix clippy collapsible-if by using `if let` + `&&` - Use HashMap for O(1) dedup instead of HashSet + linear scan - Add --user flag to `tool info` for checking non-default user credentials - Show "? unknown" on secrets store errors instead of silently reporting missing - Surface secrets store init failure via eprintln instead of silent .ok() - Sort auth entries by secret name for deterministic output Co-Authored-By: Claude Opus 4.6 (1M context) * fix(cli): only filter secrets when auth section renders, add regression test When the secrets store fails to initialize, the Auth section is not rendered. Previously, secret names were still filtered from the Secrets section, causing credential names to disappear entirely. Now secrets are only filtered when the Auth section will actually be displayed. Adds test verifying auth secret deduplication across auth, setup, and http.credentials sections, plus secrets store existence checks. Co-Authored-By: Claude Opus 4.6 (1M context) * refactor(cli): extract collect_auth_secrets helper, always render Auth section Address review feedback: - Extract dedup logic into `collect_auth_secrets()` so the test exercises the same code path as production (not a re-implementation) - Always render the Auth section when auth secrets exist, showing "? unknown" status when the secrets store is unavailable instead of hiding credential names entirely - Lazily init secrets store only when capabilities contain auth secrets, avoiding spurious warnings for tools with no auth - Add test for empty capabilities edge case Co-Authored-By: Claude Opus 4.6 (1M context) * style(cli): move HashMap/HashSet imports to top of file Co-Authored-By: Claude Opus 4.6 (1M context) * fix(cli): use correct tagged JSON format for credential location in test The CredentialLocationSchema uses serde tagged enum format ({"type": "bearer"}), not a bare string ("AuthorizationBearer"). Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- src/cli/tool.rs | 286 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 270 insertions(+), 16 deletions(-) diff --git a/src/cli/tool.rs b/src/cli/tool.rs index be684580..9d39c492 100644 --- a/src/cli/tool.rs +++ b/src/cli/tool.rs @@ -2,6 +2,7 @@ //! //! Commands for installing, listing, removing, and authenticating WASM tools. +use std::collections::{HashMap, HashSet}; use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -79,6 +80,10 @@ pub enum ToolCommand { /// Directory to look for tool (default: ~/.ironclaw/tools/) #[arg(short, long)] dir: Option, + + /// User ID for checking credential status (default: "default") + #[arg(short, long, default_value = "default")] + user: String, }, /// Configure authentication for a tool @@ -124,7 +129,11 @@ pub async fn run_tool_command(cmd: ToolCommand) -> anyhow::Result<()> { } => install_tool(path, name, capabilities, target, release, skip_build, force).await, ToolCommand::List { dir, verbose } => list_tools(dir, verbose).await, ToolCommand::Remove { name, dir } => remove_tool(name, dir).await, - ToolCommand::Info { name_or_path, dir } => show_tool_info(name_or_path, dir).await, + ToolCommand::Info { + name_or_path, + dir, + user, + } => show_tool_info(name_or_path, dir, user).await, ToolCommand::Auth { name, dir, user } => auth_tool(name, dir, user).await, ToolCommand::Setup { name, dir, user } => setup_tool(name, dir, user).await, } @@ -388,7 +397,11 @@ async fn remove_tool(name: String, dir: Option) -> anyhow::Result<()> { } /// Show information about a tool. -async fn show_tool_info(name_or_path: String, dir: Option) -> anyhow::Result<()> { +async fn show_tool_info( + name_or_path: String, + dir: Option, + user_id: String, +) -> anyhow::Result<()> { let wasm_path = if name_or_path.ends_with(".wasm") { PathBuf::from(&name_or_path) } else { @@ -423,7 +436,37 @@ async fn show_tool_info(name_or_path: String, dir: Option) -> anyhow::R println!("\nCapabilities ({}):", caps_path.display()); let content = fs::read_to_string(&caps_path).await?; match CapabilitiesFile::from_json(&content) { - Ok(caps) => print_capabilities_detail(&caps), + Ok(caps) => { + // Lazily init secrets store only when auth secrets need checking. + let has_auth = caps.auth.is_some() + || caps + .setup + .as_ref() + .is_some_and(|s| !s.required_secrets.is_empty()) + || caps + .http + .as_ref() + .is_some_and(|h| !h.credentials.is_empty()); + let secrets_store = if has_auth { + match init_secrets_store().await { + Ok(store) => Some(store), + Err(e) => { + eprintln!(" Warning: could not init secrets store: {}", e); + None + } + } + } else { + None + }; + print_capabilities_detail( + &caps, + secrets_store + .as_ref() + .map(|s| s.as_ref() as &(dyn SecretsStore + Send + Sync)), + &user_id, + ) + .await; + } Err(e) => println!(" Error parsing: {}", e), } } else { @@ -476,8 +519,89 @@ fn print_capabilities_summary(caps: &CapabilitiesFile) { } } +/// Per-secret info collected from all auth-related capability sections. +struct AuthSecretInfo { + secret_name: String, + /// Human-readable label (from auth.display_name or setup prompt). + description: Option, + /// Injection location (from http.credentials). + location: Option, +} + +/// Collected auth secrets and the set of secret names they cover. +struct CollectedAuthSecrets { + secrets: Vec, + /// Secret names present in `secrets`, for filtering the Secrets capability section. + seen_names: HashSet, +} + +/// Collect and deduplicate auth secrets from all auth-related capability sections. +/// +/// Priority for the description label: auth.display_name > setup.required_secrets.prompt. +/// Injection location is merged from http.credentials. +fn collect_auth_secrets(caps: &CapabilitiesFile) -> CollectedAuthSecrets { + let mut secrets: Vec = Vec::new(); + let mut seen: HashMap = HashMap::new(); + + // auth.display_name is the best label — seed first. + if let Some(ref auth) = caps.auth { + let index = secrets.len(); + seen.insert(auth.secret_name.clone(), index); + secrets.push(AuthSecretInfo { + secret_name: auth.secret_name.clone(), + description: auth.display_name.clone(), + location: None, + }); + } + + // setup.required_secrets.prompt is second-best label. + if let Some(ref setup) = caps.setup { + for secret in &setup.required_secrets { + if !seen.contains_key(&secret.name) { + let index = secrets.len(); + seen.insert(secret.name.clone(), index); + secrets.push(AuthSecretInfo { + secret_name: secret.name.clone(), + description: Some(secret.prompt.clone()), + location: None, + }); + } + } + } + + // Merge injection location from http.credentials. + if let Some(ref http) = caps.http { + for cred in http.credentials.values() { + let loc = format!("{:?}", cred.location); + if let Some(&index) = seen.get(&cred.secret_name) { + secrets[index].location = Some(loc); + } else { + let index = secrets.len(); + seen.insert(cred.secret_name.clone(), index); + secrets.push(AuthSecretInfo { + secret_name: cred.secret_name.clone(), + description: None, + location: Some(loc), + }); + } + } + } + + let seen_names = seen.into_keys().collect(); + CollectedAuthSecrets { + secrets, + seen_names, + } +} + /// Print detailed capabilities. -fn print_capabilities_detail(caps: &CapabilitiesFile) { +async fn print_capabilities_detail( + caps: &CapabilitiesFile, + secrets_store: Option<&(dyn SecretsStore + Send + Sync)>, + user_id: &str, +) { + let mut collected = collect_auth_secrets(caps); + if let Some(ref http) = caps.http { println!(" HTTP:"); for endpoint in &http.allowlist { @@ -490,13 +614,6 @@ fn print_capabilities_detail(caps: &CapabilitiesFile) { println!(" {} {} {}", methods, endpoint.host, path); } - if !http.credentials.is_empty() { - println!(" Credentials:"); - for (key, cred) in &http.credentials { - println!(" {}: {} -> {:?}", key, cred.secret_name, cred.location); - } - } - if let Some(ref rate) = http.rate_limit { println!( " Rate limit: {}/min, {}/hour", @@ -505,12 +622,24 @@ fn print_capabilities_detail(caps: &CapabilitiesFile) { } } + // Filter secrets already covered by the auth section (always rendered when non-empty). if let Some(ref secrets) = caps.secrets && !secrets.allowed_names.is_empty() { - println!(" Secrets (existence check only):"); - for name in &secrets.allowed_names { - println!(" {}", name); + let extra: Vec<_> = if collected.secrets.is_empty() { + secrets.allowed_names.iter().collect() + } else { + secrets + .allowed_names + .iter() + .filter(|name| !collected.seen_names.contains(name.as_str())) + .collect() + }; + if !extra.is_empty() { + println!(" Secrets (existence check only):"); + for name in extra { + println!(" {}", name); + } } } @@ -531,6 +660,38 @@ fn print_capabilities_detail(caps: &CapabilitiesFile) { println!(" {}", prefix); } } + + // Consolidated auth status — sorted by secret name for deterministic output. + if !collected.secrets.is_empty() { + collected + .secrets + .sort_by(|a, b| a.secret_name.cmp(&b.secret_name)); + println!(" Auth:"); + for info in &collected.secrets { + let (icon, label) = match secrets_store { + Some(store) => match store.exists(user_id, &info.secret_name).await { + Ok(true) => ("\u{2713}", "configured"), + Ok(false) => ("\u{2717}", "missing"), + Err(e) => { + eprintln!( + " Warning: failed to check secret `{}`: {}", + info.secret_name, e + ); + ("?", "unknown") + } + }, + None => ("?", "unknown"), + }; + let mut parts = info.secret_name.clone(); + if let Some(ref desc) = info.description { + parts = format!("{} ({})", parts, desc); + } + if let Some(ref loc) = info.location { + parts = format!("{} -> {}", parts, loc); + } + println!(" {} {} {}", parts, icon, label); + } + } } /// Validate a tool name to prevent path traversal. @@ -677,8 +838,7 @@ async fn combine_provider_scopes( secret_name: &str, base_oauth: &crate::tools::wasm::OAuthConfigSchema, ) -> crate::tools::wasm::OAuthConfigSchema { - let mut all_scopes: std::collections::HashSet = - base_oauth.scopes.iter().cloned().collect(); + let mut all_scopes: HashSet = base_oauth.scopes.iter().cloned().collect(); if let Ok(mut entries) = tokio::fs::read_dir(tools_dir).await { while let Ok(Some(entry)) = entries.next_entry().await { @@ -1127,6 +1287,8 @@ async fn setup_tool(name: String, dir: Option, user_id: String) -> anyh #[cfg(test)] mod tests { use super::*; + use crate::secrets::{CreateSecretParams, SecretsStore}; + use crate::testing::credentials::test_secrets_store; #[test] fn test_format_size() { @@ -1143,4 +1305,96 @@ mod tests { assert!(dir.to_string_lossy().contains(".ironclaw")); assert!(dir.to_string_lossy().contains("tools")); } + + /// Verify that auth secrets are deduplicated across auth, setup, and http.credentials, + /// and that credential status is checked against the secrets store. + #[tokio::test] + async fn test_auth_secret_dedup_and_status() { + let caps = CapabilitiesFile::from_json( + r#"{ + "auth": { + "secret_name": "gh_token", + "display_name": "GitHub" + }, + "setup": { + "required_secrets": [ + { "name": "gh_token", "prompt": "GitHub PAT" }, + { "name": "extra_key", "prompt": "Extra API Key" } + ] + }, + "http": { + "allowlist": [{ "host": "api.github.com" }], + "credentials": { + "github": { + "secret_name": "gh_token", + "location": { "type": "bearer" }, + "host_patterns": ["api.github.com"] + } + } + }, + "secrets": { + "allowed_names": ["gh_token", "gh_*"] + } + }"#, + ) + .unwrap(); + + let collected = collect_auth_secrets(&caps); + + // gh_token should appear once (from auth), with location merged from credentials. + // extra_key should appear once (from setup). + assert_eq!(collected.secrets.len(), 2); + let gh = collected + .secrets + .iter() + .find(|s| s.secret_name == "gh_token") + .unwrap(); + assert_eq!(gh.description.as_deref(), Some("GitHub")); + assert!( + gh.location.is_some(), + "location should be merged from http.credentials" + ); + + let extra = collected + .secrets + .iter() + .find(|s| s.secret_name == "extra_key") + .unwrap(); + assert_eq!(extra.description.as_deref(), Some("Extra API Key")); + assert!(extra.location.is_none()); + + // Secrets section should filter gh_token (in seen_names) but keep gh_* (wildcard). + let secrets = caps.secrets.as_ref().unwrap(); + let extra_secrets: Vec<_> = secrets + .allowed_names + .iter() + .filter(|name| !collected.seen_names.contains(name.as_str())) + .collect(); + assert_eq!(extra_secrets, vec!["gh_*"]); + + // Verify store check: missing secret -> exists returns false. + let store = test_secrets_store(); + assert!(!store.exists("default", "gh_token").await.unwrap()); + + // Store gh_token and verify it's found. + store + .create( + "default", + CreateSecretParams::new("gh_token", "ghp_test123"), + ) + .await + .unwrap(); + assert!(store.exists("default", "gh_token").await.unwrap()); + // extra_key still missing. + assert!(!store.exists("default", "extra_key").await.unwrap()); + } + + /// No auth sections → collect_auth_secrets returns empty. + #[test] + fn test_collect_auth_secrets_empty_caps() { + let caps = CapabilitiesFile::default(); + let collected = collect_auth_secrets(&caps); + assert!(collected.secrets.is_empty()); + assert!(collected.seen_names.is_empty()); + } } From 706c3a1b4747d0335fd45013deddde3239be2f7f Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Tue, 24 Mar 2026 23:02:46 -0700 Subject: [PATCH 2/8] refactor: extract AppEvent to crates/ironclaw_common (#1615) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: extract AppEvent to crates/ironclaw_common SseEvent was defined in src/channels/web/types.rs but imported by 12+ modules across agent, orchestrator, worker, tools, and extensions — it had become the application-wide event protocol, not a web transport concern. Create crates/ironclaw_common as a shared workspace crate and move the enum there as AppEvent. Also move the truncate_preview utility which was similarly leaked from the web gateway into agent modules. - New crate: crates/ironclaw_common (AppEvent, truncate_preview) - Rename SseEvent → AppEvent, from_sse_event → from_app_event - web/types.rs re-exports AppEvent for internal gateway use - web/util.rs re-exports truncate_preview - Wire format unchanged (serde renames are on variants, not the enum) Aligned with the event bus direction on refactor/architectural-hardening where DomainEvent (≡ AppEvent) is wrapped in a SystemEvent envelope. Co-Authored-By: Claude Opus 4.6 (1M context) * refactor: add AppEvent::event_type() helper, deduplicate match blocks Address Gemini review: extract the variant→string match into a single method on AppEvent, replacing the duplicated 22-arm matches in sse.rs and types.rs. Co-Authored-By: Claude Opus 4.6 (1M context) * refactor: rename leftover sse vars/tests to match AppEvent rename Address Copilot review: rename sse_event vars to app_event in orchestrator/api.rs and ws.rs, rename test functions from test_ws_server_from_sse_* to test_ws_server_from_app_event_*, and update stale SSE comments. Co-Authored-By: Claude Opus 4.6 (1M context) * refactor: add Deserialize to AppEvent, round-trip test, fix stale comments Address zmanian review: - Add Deserialize derive to AppEvent so downstream consumers can deserialize incoming events - Add event_type_matches_serde_type_field test that round-trips every variant through serde and asserts event_type() matches the serialized "type" field — catches drift between serde renames and the manual match - Add round_trip_deserialize test for basic Serialize/Deserialize parity - Update remaining "SSE" references in comments across server.rs, manager.rs, ws_gateway_integration.rs, and worker/job.rs Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- Cargo.lock | 9 + Cargo.toml | 5 +- crates/ironclaw_common/Cargo.toml | 18 ++ crates/ironclaw_common/src/event.rs | 338 ++++++++++++++++++++++++++++ crates/ironclaw_common/src/lib.rs | 7 + crates/ironclaw_common/src/util.rs | 100 ++++++++ src/agent/job_monitor.rs | 48 ++-- src/agent/session.rs | 2 +- src/agent/thread_ops.rs | 2 +- src/channels/web/handlers/chat.rs | 6 +- src/channels/web/mod.rs | 32 +-- src/channels/web/server.rs | 24 +- src/channels/web/sse.rs | 61 ++--- src/channels/web/types.rs | 233 +++---------------- src/channels/web/util.rs | 106 +-------- src/channels/web/ws.rs | 8 +- src/extensions/manager.rs | 6 +- src/orchestrator/api.rs | 28 +-- src/orchestrator/mod.rs | 4 +- src/tools/builtin/job.rs | 6 +- src/tools/registry.rs | 6 +- src/worker/job.rs | 14 +- tests/multi_tenant_integration.rs | 46 ++-- tests/ws_gateway_integration.rs | 18 +- 24 files changed, 646 insertions(+), 481 deletions(-) create mode 100644 crates/ironclaw_common/Cargo.toml create mode 100644 crates/ironclaw_common/src/event.rs create mode 100644 crates/ironclaw_common/src/lib.rs create mode 100644 crates/ironclaw_common/src/util.rs diff --git a/Cargo.lock b/Cargo.lock index a813ef2b..27c258c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3428,6 +3428,7 @@ dependencies = [ "hyper-util", "iana-time-zone", "insta", + "ironclaw_common", "ironclaw_safety", "json5", "libsql", @@ -3485,6 +3486,14 @@ dependencies = [ "zip", ] +[[package]] +name = "ironclaw_common" +version = "0.1.0" +dependencies = [ + "serde", + "serde_json", +] + [[package]] name = "ironclaw_safety" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 99992a40..395e42d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [workspace] -members = [".", "crates/ironclaw_safety"] +members = [".", "crates/ironclaw_common", "crates/ironclaw_safety"] exclude = [ "channels-src/discord", "channels-src/telegram", @@ -100,6 +100,9 @@ tower-http = { version = "0.6", features = ["trace", "cors", "set-header"] } # Cron scheduling for routines cron = "0.13" +# Shared types +ironclaw_common = { path = "crates/ironclaw_common", version = "0.1.0" } + # Safety/sanitization ironclaw_safety = { path = "crates/ironclaw_safety", version = "0.1.0" } regex = "1" diff --git a/crates/ironclaw_common/Cargo.toml b/crates/ironclaw_common/Cargo.toml new file mode 100644 index 00000000..353ab747 --- /dev/null +++ b/crates/ironclaw_common/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "ironclaw_common" +version = "0.1.0" +edition = "2024" +rust-version = "1.92" +description = "Shared types and utilities for the IronClaw workspace" +authors = ["NEAR AI "] +license = "MIT OR Apache-2.0" +homepage = "https://github.com/nearai/ironclaw" +repository = "https://github.com/nearai/ironclaw" +publish = false + +[package.metadata.dist] +dist = false + +[dependencies] +serde = { version = "1", features = ["derive"] } +serde_json = "1" diff --git a/crates/ironclaw_common/src/event.rs b/crates/ironclaw_common/src/event.rs new file mode 100644 index 00000000..83592c95 --- /dev/null +++ b/crates/ironclaw_common/src/event.rs @@ -0,0 +1,338 @@ +//! Application-wide event types. +//! +//! `AppEvent` is the real-time event protocol used across the entire +//! application. The web gateway serialises these to SSE / WebSocket +//! frames, but other subsystems (agent loop, orchestrator, extensions) +//! produce and consume them too. + +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "type")] +pub enum AppEvent { + #[serde(rename = "response")] + Response { content: String, thread_id: String }, + #[serde(rename = "thinking")] + Thinking { + message: String, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + #[serde(rename = "tool_started")] + ToolStarted { + name: String, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + #[serde(rename = "tool_completed")] + ToolCompleted { + name: String, + success: bool, + #[serde(skip_serializing_if = "Option::is_none")] + error: Option, + #[serde(skip_serializing_if = "Option::is_none")] + parameters: Option, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + #[serde(rename = "tool_result")] + ToolResult { + name: String, + preview: String, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + #[serde(rename = "stream_chunk")] + StreamChunk { + content: String, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + #[serde(rename = "status")] + Status { + message: String, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + #[serde(rename = "job_started")] + JobStarted { + job_id: String, + title: String, + browse_url: String, + }, + #[serde(rename = "approval_needed")] + ApprovalNeeded { + request_id: String, + tool_name: String, + description: String, + parameters: String, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + /// Whether the "always" auto-approve option should be shown. + allow_always: bool, + }, + #[serde(rename = "auth_required")] + AuthRequired { + extension_name: String, + #[serde(skip_serializing_if = "Option::is_none")] + instructions: Option, + #[serde(skip_serializing_if = "Option::is_none")] + auth_url: Option, + #[serde(skip_serializing_if = "Option::is_none")] + setup_url: Option, + }, + #[serde(rename = "auth_completed")] + AuthCompleted { + extension_name: String, + success: bool, + message: String, + }, + #[serde(rename = "error")] + Error { + message: String, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + #[serde(rename = "heartbeat")] + Heartbeat, + + // Sandbox job streaming events (worker + Claude Code bridge) + #[serde(rename = "job_message")] + JobMessage { + job_id: String, + role: String, + content: String, + }, + #[serde(rename = "job_tool_use")] + JobToolUse { + job_id: String, + tool_name: String, + input: serde_json::Value, + }, + #[serde(rename = "job_tool_result")] + JobToolResult { + job_id: String, + tool_name: String, + output: String, + }, + #[serde(rename = "job_status")] + JobStatus { job_id: String, message: String }, + #[serde(rename = "job_result")] + JobResult { + job_id: String, + status: String, + #[serde(skip_serializing_if = "Option::is_none")] + session_id: Option, + #[serde(skip_serializing_if = "Option::is_none")] + fallback_deliverable: Option, + }, + + /// An image was generated by a tool. + #[serde(rename = "image_generated")] + ImageGenerated { + data_url: String, + #[serde(skip_serializing_if = "Option::is_none")] + path: Option, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + + /// Suggested follow-up messages for the user. + #[serde(rename = "suggestions")] + Suggestions { + suggestions: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + + /// Per-turn token usage and cost summary. + #[serde(rename = "turn_cost")] + TurnCost { + input_tokens: u64, + output_tokens: u64, + cost_usd: String, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + + /// Extension activation status change (WASM channels). + #[serde(rename = "extension_status")] + ExtensionStatus { + extension_name: String, + status: String, + #[serde(skip_serializing_if = "Option::is_none")] + message: Option, + }, +} + +impl AppEvent { + /// The wire-format event type string (matches the `#[serde(rename)]` value). + pub fn event_type(&self) -> &'static str { + match self { + Self::Response { .. } => "response", + Self::Thinking { .. } => "thinking", + Self::ToolStarted { .. } => "tool_started", + Self::ToolCompleted { .. } => "tool_completed", + Self::ToolResult { .. } => "tool_result", + Self::StreamChunk { .. } => "stream_chunk", + Self::Status { .. } => "status", + Self::JobStarted { .. } => "job_started", + Self::ApprovalNeeded { .. } => "approval_needed", + Self::AuthRequired { .. } => "auth_required", + Self::AuthCompleted { .. } => "auth_completed", + Self::Error { .. } => "error", + Self::Heartbeat => "heartbeat", + Self::JobMessage { .. } => "job_message", + Self::JobToolUse { .. } => "job_tool_use", + Self::JobToolResult { .. } => "job_tool_result", + Self::JobStatus { .. } => "job_status", + Self::JobResult { .. } => "job_result", + Self::ImageGenerated { .. } => "image_generated", + Self::Suggestions { .. } => "suggestions", + Self::TurnCost { .. } => "turn_cost", + Self::ExtensionStatus { .. } => "extension_status", + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Verify that `event_type()` returns the same string as the serde + /// `"type"` field for every variant. This catches drift between the + /// `#[serde(rename)]` attributes and the manual match arms. + #[test] + fn event_type_matches_serde_type_field() { + let variants: Vec = vec![ + AppEvent::Response { + content: String::new(), + thread_id: String::new(), + }, + AppEvent::Thinking { + message: String::new(), + thread_id: None, + }, + AppEvent::ToolStarted { + name: String::new(), + thread_id: None, + }, + AppEvent::ToolCompleted { + name: String::new(), + success: true, + error: None, + parameters: None, + thread_id: None, + }, + AppEvent::ToolResult { + name: String::new(), + preview: String::new(), + thread_id: None, + }, + AppEvent::StreamChunk { + content: String::new(), + thread_id: None, + }, + AppEvent::Status { + message: String::new(), + thread_id: None, + }, + AppEvent::JobStarted { + job_id: String::new(), + title: String::new(), + browse_url: String::new(), + }, + AppEvent::ApprovalNeeded { + request_id: String::new(), + tool_name: String::new(), + description: String::new(), + parameters: String::new(), + thread_id: None, + allow_always: false, + }, + AppEvent::AuthRequired { + extension_name: String::new(), + instructions: None, + auth_url: None, + setup_url: None, + }, + AppEvent::AuthCompleted { + extension_name: String::new(), + success: true, + message: String::new(), + }, + AppEvent::Error { + message: String::new(), + thread_id: None, + }, + AppEvent::Heartbeat, + AppEvent::JobMessage { + job_id: String::new(), + role: String::new(), + content: String::new(), + }, + AppEvent::JobToolUse { + job_id: String::new(), + tool_name: String::new(), + input: serde_json::Value::Null, + }, + AppEvent::JobToolResult { + job_id: String::new(), + tool_name: String::new(), + output: String::new(), + }, + AppEvent::JobStatus { + job_id: String::new(), + message: String::new(), + }, + AppEvent::JobResult { + job_id: String::new(), + status: String::new(), + session_id: None, + fallback_deliverable: None, + }, + AppEvent::ImageGenerated { + data_url: String::new(), + path: None, + thread_id: None, + }, + AppEvent::Suggestions { + suggestions: vec![], + thread_id: None, + }, + AppEvent::TurnCost { + input_tokens: 0, + output_tokens: 0, + cost_usd: String::new(), + thread_id: None, + }, + AppEvent::ExtensionStatus { + extension_name: String::new(), + status: String::new(), + message: None, + }, + ]; + + for variant in &variants { + let json: serde_json::Value = serde_json::to_value(variant).unwrap(); + let serde_type = json["type"].as_str().unwrap(); + assert_eq!( + variant.event_type(), + serde_type, + "event_type() mismatch for variant: {:?}", + variant + ); + } + } + + #[test] + fn round_trip_deserialize() { + let original = AppEvent::Response { + content: "hello".to_string(), + thread_id: "t1".to_string(), + }; + let json = serde_json::to_string(&original).unwrap(); + let deserialized: AppEvent = serde_json::from_str(&json).unwrap(); + assert_eq!(deserialized.event_type(), "response"); + } +} diff --git a/crates/ironclaw_common/src/lib.rs b/crates/ironclaw_common/src/lib.rs new file mode 100644 index 00000000..6822bad1 --- /dev/null +++ b/crates/ironclaw_common/src/lib.rs @@ -0,0 +1,7 @@ +//! Shared types and utilities for the IronClaw workspace. + +mod event; +mod util; + +pub use event::AppEvent; +pub use util::truncate_preview; diff --git a/crates/ironclaw_common/src/util.rs b/crates/ironclaw_common/src/util.rs new file mode 100644 index 00000000..4f054671 --- /dev/null +++ b/crates/ironclaw_common/src/util.rs @@ -0,0 +1,100 @@ +//! Shared utility functions. + +/// Truncate a string to at most `max_bytes` bytes at a char boundary, appending "...". +/// +/// If the input is wrapped in `...` and truncation +/// removes the closing tag, the tag is re-appended so downstream XML parsers +/// never see an unclosed element. +pub fn truncate_preview(s: &str, max_bytes: usize) -> String { + if s.len() <= max_bytes { + return s.to_string(); + } + // Walk backwards from max_bytes to find a valid char boundary + let mut end = max_bytes; + while end > 0 && !s.is_char_boundary(end) { + end -= 1; + } + let mut result = format!("{}...", &s[..end]); + + // Re-close if truncation cut through the closing tag. + if s.starts_with("") { + result.push_str("\n"); + } + + result +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_truncate_preview_short_string() { + assert_eq!(truncate_preview("hello", 10), "hello"); + } + + #[test] + fn test_truncate_preview_exact_boundary() { + assert_eq!(truncate_preview("hello", 5), "hello"); + } + + #[test] + fn test_truncate_preview_truncates_ascii() { + assert_eq!(truncate_preview("hello world", 5), "hello..."); + } + + #[test] + fn test_truncate_preview_empty_string() { + assert_eq!(truncate_preview("", 10), ""); + } + + #[test] + fn test_truncate_preview_multibyte_char_boundary() { + let s = "a\u{20AC}b"; + let result = truncate_preview(s, 3); + assert_eq!(result, "a..."); + } + + #[test] + fn test_truncate_preview_emoji() { + let s = "hi\u{1F980}"; + let result = truncate_preview(s, 4); + assert_eq!(result, "hi..."); + } + + #[test] + fn test_truncate_preview_cjk() { + let s = "\u{4F60}\u{597D}\u{4E16}\u{754C}"; + let result = truncate_preview(s, 7); + assert_eq!(result, "\u{4F60}\u{597D}..."); + } + + #[test] + fn test_truncate_preview_zero_max_bytes() { + assert_eq!(truncate_preview("hello", 0), "..."); + } + + #[test] + fn test_truncate_preview_closes_tool_output_tag() { + let s = "\nSome very long content here\n"; + let result = truncate_preview(s, 60); + assert!(result.ends_with("")); + assert!(result.contains("...")); + } + + #[test] + fn test_truncate_preview_no_extra_close_when_intact() { + let s = "\nshort\n"; + let result = truncate_preview(s, 500); + assert_eq!(result, s); + assert_eq!(result.matches("").count(), 1); + } + + #[test] + fn test_truncate_preview_non_xml_unaffected() { + let s = "Just a plain long string that gets truncated"; + let result = truncate_preview(s, 10); + assert_eq!(result, "Just a pla..."); + assert!(!result.contains("")); + } +} diff --git a/src/agent/job_monitor.rs b/src/agent/job_monitor.rs index 02f5e3e2..e102dfbf 100644 --- a/src/agent/job_monitor.rs +++ b/src/agent/job_monitor.rs @@ -21,8 +21,8 @@ use tokio::task::JoinHandle; use uuid::Uuid; use crate::channels::IncomingMessage; -use crate::channels::web::types::SseEvent; use crate::context::{ContextManager, JobState}; +use ironclaw_common::AppEvent; /// Route context for forwarding job monitor events back to the user's channel. #[derive(Debug, Clone)] @@ -36,15 +36,15 @@ pub struct JobMonitorRoute { /// injects assistant messages into the agent loop. /// /// The monitor forwards: -/// - `SseEvent::JobMessage` (assistant role): injected as incoming messages so +/// - `AppEvent::JobMessage` (assistant role): injected as incoming messages so /// the main agent can read and relay to the user. -/// - `SseEvent::JobResult`: injected as a completion notice, then the task exits. +/// - `AppEvent::JobResult`: injected as a completion notice, then the task exits. /// /// Tool use/result and status events are intentionally skipped (too noisy for /// the main agent's context window). pub fn spawn_job_monitor( job_id: Uuid, - event_rx: broadcast::Receiver<(Uuid, String, SseEvent)>, + event_rx: broadcast::Receiver<(Uuid, String, AppEvent)>, inject_tx: mpsc::Sender, route: JobMonitorRoute, ) -> JoinHandle<()> { @@ -56,7 +56,7 @@ pub fn spawn_job_monitor( /// jobs don't stay `InProgress` forever in the `ContextManager`. pub fn spawn_job_monitor_with_context( job_id: Uuid, - mut event_rx: broadcast::Receiver<(Uuid, String, SseEvent)>, + mut event_rx: broadcast::Receiver<(Uuid, String, AppEvent)>, inject_tx: mpsc::Sender, route: JobMonitorRoute, context_manager: Option>, @@ -74,7 +74,7 @@ pub fn spawn_job_monitor_with_context( } match event { - SseEvent::JobMessage { role, content, .. } if role == "assistant" => { + AppEvent::JobMessage { role, content, .. } if role == "assistant" => { let mut msg = IncomingMessage::new( route.channel.clone(), route.user_id.clone(), @@ -92,7 +92,7 @@ pub fn spawn_job_monitor_with_context( break; } } - SseEvent::JobResult { status, .. } => { + AppEvent::JobResult { status, .. } => { // Transition in-memory state so the job frees its // max_jobs slot and query tools show the final state. if let Some(ref cm) = context_manager { @@ -162,7 +162,7 @@ pub fn spawn_job_monitor_with_context( /// inject messages into) but we still need to free the `max_jobs` slot. pub fn spawn_completion_watcher( job_id: Uuid, - mut event_rx: broadcast::Receiver<(Uuid, String, SseEvent)>, + mut event_rx: broadcast::Receiver<(Uuid, String, AppEvent)>, context_manager: Arc, ) -> JoinHandle<()> { let short_id = job_id.to_string()[..8].to_string(); @@ -170,7 +170,7 @@ pub fn spawn_completion_watcher( tokio::spawn(async move { loop { match event_rx.recv().await { - Ok((ev_job_id, _user_id, SseEvent::JobResult { status, .. })) + Ok((ev_job_id, _user_id, AppEvent::JobResult { status, .. })) if ev_job_id == job_id => { let target = if status == "completed" { @@ -229,7 +229,7 @@ mod tests { #[tokio::test] async fn test_monitor_forwards_assistant_messages() { - let (event_tx, _) = broadcast::channel::<(Uuid, String, SseEvent)>(16); + let (event_tx, _) = broadcast::channel::<(Uuid, String, AppEvent)>(16); let (inject_tx, mut inject_rx) = mpsc::channel::(16); let job_id = Uuid::new_v4(); @@ -240,7 +240,7 @@ mod tests { .send(( job_id, "test-user".to_string(), - SseEvent::JobMessage { + AppEvent::JobMessage { job_id: job_id.to_string(), role: "assistant".to_string(), content: "I found a bug".to_string(), @@ -262,7 +262,7 @@ mod tests { #[tokio::test] async fn test_monitor_ignores_other_jobs() { - let (event_tx, _) = broadcast::channel::<(Uuid, String, SseEvent)>(16); + let (event_tx, _) = broadcast::channel::<(Uuid, String, AppEvent)>(16); let (inject_tx, mut inject_rx) = mpsc::channel::(16); let job_id = Uuid::new_v4(); @@ -274,7 +274,7 @@ mod tests { .send(( other_job_id, "test-user".to_string(), - SseEvent::JobMessage { + AppEvent::JobMessage { job_id: other_job_id.to_string(), role: "assistant".to_string(), content: "wrong job".to_string(), @@ -293,7 +293,7 @@ mod tests { #[tokio::test] async fn test_monitor_exits_on_job_result() { - let (event_tx, _) = broadcast::channel::<(Uuid, String, SseEvent)>(16); + let (event_tx, _) = broadcast::channel::<(Uuid, String, AppEvent)>(16); let (inject_tx, mut inject_rx) = mpsc::channel::(16); let job_id = Uuid::new_v4(); @@ -304,7 +304,7 @@ mod tests { .send(( job_id, "test-user".to_string(), - SseEvent::JobResult { + AppEvent::JobResult { job_id: job_id.to_string(), status: "completed".to_string(), session_id: None, @@ -329,7 +329,7 @@ mod tests { #[tokio::test] async fn test_monitor_skips_tool_events() { - let (event_tx, _) = broadcast::channel::<(Uuid, String, SseEvent)>(16); + let (event_tx, _) = broadcast::channel::<(Uuid, String, AppEvent)>(16); let (inject_tx, mut inject_rx) = mpsc::channel::(16); let job_id = Uuid::new_v4(); @@ -340,7 +340,7 @@ mod tests { .send(( job_id, "test-user".to_string(), - SseEvent::JobToolUse { + AppEvent::JobToolUse { job_id: job_id.to_string(), tool_name: "shell".to_string(), input: serde_json::json!({"command": "ls"}), @@ -353,7 +353,7 @@ mod tests { .send(( job_id, "test-user".to_string(), - SseEvent::JobMessage { + AppEvent::JobMessage { job_id: job_id.to_string(), role: "user".to_string(), content: "user prompt".to_string(), @@ -409,7 +409,7 @@ mod tests { .await .unwrap(); - let (event_tx, _) = broadcast::channel::<(Uuid, String, SseEvent)>(16); + let (event_tx, _) = broadcast::channel::<(Uuid, String, AppEvent)>(16); let (inject_tx, mut inject_rx) = mpsc::channel::(16); let handle = spawn_job_monitor_with_context( @@ -425,7 +425,7 @@ mod tests { .send(( job_id, "test-user".to_string(), - SseEvent::JobResult { + AppEvent::JobResult { job_id: job_id.to_string(), status: "completed".to_string(), session_id: None, @@ -458,7 +458,7 @@ mod tests { .await .unwrap(); - let (event_tx, _) = broadcast::channel::<(Uuid, String, SseEvent)>(16); + let (event_tx, _) = broadcast::channel::<(Uuid, String, AppEvent)>(16); let (inject_tx, mut inject_rx) = mpsc::channel::(16); let handle = spawn_job_monitor_with_context( @@ -474,7 +474,7 @@ mod tests { .send(( job_id, "test-user".to_string(), - SseEvent::JobResult { + AppEvent::JobResult { job_id: job_id.to_string(), status: "failed".to_string(), session_id: None, @@ -507,14 +507,14 @@ mod tests { .await .unwrap(); - let (event_tx, _) = broadcast::channel::<(Uuid, String, SseEvent)>(16); + let (event_tx, _) = broadcast::channel::<(Uuid, String, AppEvent)>(16); let handle = spawn_completion_watcher(job_id, event_tx.subscribe(), Arc::clone(&cm)); event_tx .send(( job_id, "test-user".to_string(), - SseEvent::JobResult { + AppEvent::JobResult { job_id: job_id.to_string(), status: "completed".to_string(), session_id: None, diff --git a/src/agent/session.rs b/src/agent/session.rs index 45594922..7ec2023f 100644 --- a/src/agent/session.rs +++ b/src/agent/session.rs @@ -16,8 +16,8 @@ use chrono::{DateTime, TimeDelta, Utc}; use serde::{Deserialize, Serialize}; use uuid::Uuid; -use crate::channels::web::util::truncate_preview; use crate::llm::{ChatMessage, ToolCall, generate_tool_call_id}; +use ironclaw_common::truncate_preview; /// A session containing one or more threads. #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index ddfd0c0f..b2820e7e 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -16,12 +16,12 @@ use crate::agent::dispatcher::{ }; use crate::agent::session::{MAX_PENDING_MESSAGES, PendingApproval, Session, ThreadState}; use crate::agent::submission::SubmissionResult; -use crate::channels::web::util::truncate_preview; use crate::channels::{IncomingMessage, StatusUpdate}; use crate::context::JobContext; use crate::error::Error; use crate::llm::{ChatMessage, ToolCall}; use crate::tools::redact_params; +use ironclaw_common::truncate_preview; const FORGED_THREAD_ID_ERROR: &str = "Invalid or unauthorized thread ID."; diff --git a/src/channels/web/handlers/chat.rs b/src/channels/web/handlers/chat.rs index 9753c015..de4b3155 100644 --- a/src/channels/web/handlers/chat.rs +++ b/src/channels/web/handlers/chat.rs @@ -175,7 +175,7 @@ pub async fn chat_auth_token_handler( if result.verification.is_some() { state.sse.broadcast_for_user( &user.user_id, - SseEvent::AuthRequired { + AppEvent::AuthRequired { extension_name: req.extension_name.clone(), instructions: Some(result.message), auth_url: None, @@ -187,7 +187,7 @@ pub async fn chat_auth_token_handler( state.sse.broadcast_for_user( &user.user_id, - SseEvent::AuthCompleted { + AppEvent::AuthCompleted { extension_name: req.extension_name.clone(), success: true, message: result.message, @@ -202,7 +202,7 @@ pub async fn chat_auth_token_handler( if matches!(e, crate::extensions::ExtensionError::ValidationFailed(_)) { state.sse.broadcast_for_user( &user.user_id, - SseEvent::AuthRequired { + AppEvent::AuthRequired { extension_name: req.extension_name.clone(), instructions: Some(msg.clone()), auth_url: None, diff --git a/src/channels/web/mod.rs b/src/channels/web/mod.rs index a8b1ec41..6a97e8b8 100644 --- a/src/channels/web/mod.rs +++ b/src/channels/web/mod.rs @@ -58,7 +58,7 @@ use self::log_layer::{LogBroadcaster, LogLevelHandle}; use self::auth::MultiAuthState; use self::server::GatewayState; use self::sse::SseManager; -use self::types::SseEvent; +use self::types::AppEvent; /// Web gateway channel implementing the Channel trait. pub struct GatewayChannel { @@ -386,7 +386,7 @@ impl Channel for GatewayChannel { self.state.sse.broadcast_for_user( &msg.user_id, - SseEvent::Response { + AppEvent::Response { content: response.content, thread_id, }, @@ -405,11 +405,11 @@ impl Channel for GatewayChannel { .and_then(|v| v.as_str()) .map(String::from); let event = match status { - StatusUpdate::Thinking(msg) => SseEvent::Thinking { + StatusUpdate::Thinking(msg) => AppEvent::Thinking { message: msg, thread_id: thread_id.clone(), }, - StatusUpdate::ToolStarted { name } => SseEvent::ToolStarted { + StatusUpdate::ToolStarted { name } => AppEvent::ToolStarted { name, thread_id: thread_id.clone(), }, @@ -418,23 +418,23 @@ impl Channel for GatewayChannel { success, error, parameters, - } => SseEvent::ToolCompleted { + } => AppEvent::ToolCompleted { name, success, error, parameters, thread_id: thread_id.clone(), }, - StatusUpdate::ToolResult { name, preview } => SseEvent::ToolResult { + StatusUpdate::ToolResult { name, preview } => AppEvent::ToolResult { name, preview, thread_id: thread_id.clone(), }, - StatusUpdate::StreamChunk(content) => SseEvent::StreamChunk { + StatusUpdate::StreamChunk(content) => AppEvent::StreamChunk { content, thread_id: thread_id.clone(), }, - StatusUpdate::Status(msg) => SseEvent::Status { + StatusUpdate::Status(msg) => AppEvent::Status { message: msg, thread_id: thread_id.clone(), }, @@ -442,7 +442,7 @@ impl Channel for GatewayChannel { job_id, title, browse_url, - } => SseEvent::JobStarted { + } => AppEvent::JobStarted { job_id, title, browse_url, @@ -453,7 +453,7 @@ impl Channel for GatewayChannel { description, parameters, allow_always, - } => SseEvent::ApprovalNeeded { + } => AppEvent::ApprovalNeeded { request_id, tool_name, description, @@ -467,7 +467,7 @@ impl Channel for GatewayChannel { instructions, auth_url, setup_url, - } => SseEvent::AuthRequired { + } => AppEvent::AuthRequired { extension_name, instructions, auth_url, @@ -477,17 +477,17 @@ impl Channel for GatewayChannel { extension_name, success, message, - } => SseEvent::AuthCompleted { + } => AppEvent::AuthCompleted { extension_name, success, message, }, - StatusUpdate::ImageGenerated { data_url, path } => SseEvent::ImageGenerated { + StatusUpdate::ImageGenerated { data_url, path } => AppEvent::ImageGenerated { data_url, path, thread_id: thread_id.clone(), }, - StatusUpdate::Suggestions { suggestions } => SseEvent::Suggestions { + StatusUpdate::Suggestions { suggestions } => AppEvent::Suggestions { suggestions, thread_id, }, @@ -495,7 +495,7 @@ impl Channel for GatewayChannel { input_tokens, output_tokens, cost_usd, - } => SseEvent::TurnCost { + } => AppEvent::TurnCost { input_tokens, output_tokens, cost_usd, @@ -531,7 +531,7 @@ impl Channel for GatewayChannel { }; self.state.sse.broadcast_for_user( user_id, - SseEvent::Response { + AppEvent::Response { content: response.content, thread_id, }, diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 31c2b296..5b092312 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -813,7 +813,7 @@ async fn oauth_callback_handler( if let Some(ref sse) = flow.sse_manager { sse.broadcast_for_user( &flow.user_id, - SseEvent::AuthCompleted { + AppEvent::AuthCompleted { extension_name: flow.extension_name.clone(), success: false, message: "OAuth flow expired. Please try again.".to_string(), @@ -951,11 +951,11 @@ async fn oauth_callback_handler( message }; - // Broadcast SSE event to notify the web UI + // Broadcast event to notify the web UI if let Some(ref sse) = flow.sse_manager { sse.broadcast_for_user( &flow.user_id, - SseEvent::AuthCompleted { + AppEvent::AuthCompleted { extension_name: flow.extension_name, success, message: final_message.clone(), @@ -1197,8 +1197,8 @@ async fn slack_relay_oauth_callback_handler( } }; - // Broadcast SSE event to notify the web UI - state.sse.broadcast(SseEvent::AuthCompleted { + // Broadcast event to notify the web UI + state.sse.broadcast(AppEvent::AuthCompleted { extension_name: DEFAULT_RELAY_NAME.to_string(), success, message: message.clone(), @@ -1471,7 +1471,7 @@ async fn chat_auth_token_handler( if result.verification.is_some() { state.sse.broadcast_for_user( &user.user_id, - SseEvent::AuthRequired { + AppEvent::AuthRequired { extension_name: req.extension_name.clone(), instructions: Some(result.message), auth_url: None, @@ -1484,7 +1484,7 @@ async fn chat_auth_token_handler( state.sse.broadcast_for_user( &user.user_id, - SseEvent::AuthCompleted { + AppEvent::AuthCompleted { extension_name: req.extension_name.clone(), success: true, message: result.message, @@ -1493,7 +1493,7 @@ async fn chat_auth_token_handler( } else { state.sse.broadcast_for_user( &user.user_id, - SseEvent::AuthCompleted { + AppEvent::AuthCompleted { extension_name: req.extension_name.clone(), success: false, message: result.message, @@ -1509,7 +1509,7 @@ async fn chat_auth_token_handler( if matches!(e, crate::extensions::ExtensionError::ValidationFailed(_)) { state.sse.broadcast_for_user( &user.user_id, - SseEvent::AuthRequired { + AppEvent::AuthRequired { extension_name: req.extension_name.clone(), instructions: Some(msg.clone()), auth_url: None, @@ -2477,7 +2477,7 @@ async fn extensions_setup_submit_handler( // auth card or setup modal that was triggered by tool_auth/tool_activate. state.sse.broadcast_for_user( &user.user_id, - SseEvent::AuthCompleted { + AppEvent::AuthCompleted { extension_name: name.clone(), success: result.activated, message: resp.message.clone(), @@ -3169,7 +3169,7 @@ mod tests { Ok(Ok(scoped)) if matches!( scoped.event, - crate::channels::web::types::SseEvent::AuthRequired { .. } + crate::channels::web::types::AppEvent::AuthRequired { .. } ) => { panic!("verification responses should not emit auth_required SSE events") @@ -3451,7 +3451,7 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); match receiver.recv().await.expect("auth_completed event").event { - crate::channels::web::types::SseEvent::AuthCompleted { + crate::channels::web::types::AppEvent::AuthCompleted { extension_name, success, message, diff --git a/src/channels/web/sse.rs b/src/channels/web/sse.rs index 46841e19..e36cceab 100644 --- a/src/channels/web/sse.rs +++ b/src/channels/web/sse.rs @@ -11,7 +11,7 @@ use tokio::sync::broadcast; use tokio_stream::StreamExt; use tokio_stream::wrappers::BroadcastStream; -use crate::channels::web::types::SseEvent; +use crate::channels::web::types::AppEvent; /// Maximum number of concurrent SSE/WebSocket connections. /// Prevents resource exhaustion from connection flooding. @@ -25,7 +25,7 @@ const MAX_CONNECTIONS: u64 = 100; #[derive(Debug, Clone)] pub(crate) struct ScopedEvent { pub(crate) user_id: Option, - pub(crate) event: SseEvent, + pub(crate) event: AppEvent, } /// Manages SSE broadcast to all connected browser tabs. @@ -75,7 +75,7 @@ impl SseManager { } /// Broadcast an event to all connected clients (global/unscoped). - pub fn broadcast(&self, event: SseEvent) { + pub fn broadcast(&self, event: AppEvent) { let _ = self.tx.send(ScopedEvent { user_id: None, event, @@ -86,7 +86,7 @@ impl SseManager { /// /// Only subscribers for this user_id (or unscoped subscribers) will /// receive the event. - pub fn broadcast_for_user(&self, user_id: &str, event: SseEvent) { + pub fn broadcast_for_user(&self, user_id: &str, event: AppEvent) { let _ = self.tx.send(ScopedEvent { user_id: Some(user_id.to_string()), event, @@ -108,7 +108,7 @@ impl SseManager { pub fn subscribe_raw( &self, user_id: Option, - ) -> Option + Send + 'static + use<>> { + ) -> Option + Send + 'static + use<>> { // Atomically increment only if below the limit. This prevents // concurrent callers from overshooting max_connections. let counter = Arc::clone(&self.connection_count); @@ -186,30 +186,7 @@ impl SseManager { return None; } }; - let event_type = match &event { - SseEvent::Response { .. } => "response", - SseEvent::Thinking { .. } => "thinking", - SseEvent::ToolStarted { .. } => "tool_started", - SseEvent::ToolCompleted { .. } => "tool_completed", - SseEvent::ToolResult { .. } => "tool_result", - SseEvent::StreamChunk { .. } => "stream_chunk", - SseEvent::Status { .. } => "status", - SseEvent::ApprovalNeeded { .. } => "approval_needed", - SseEvent::AuthRequired { .. } => "auth_required", - SseEvent::AuthCompleted { .. } => "auth_completed", - SseEvent::Error { .. } => "error", - SseEvent::JobStarted { .. } => "job_started", - SseEvent::JobMessage { .. } => "job_message", - SseEvent::JobToolUse { .. } => "job_tool_use", - SseEvent::JobToolResult { .. } => "job_tool_result", - SseEvent::JobStatus { .. } => "job_status", - SseEvent::JobResult { .. } => "job_result", - SseEvent::Heartbeat => "heartbeat", - SseEvent::ImageGenerated { .. } => "image_generated", - SseEvent::Suggestions { .. } => "suggestions", - SseEvent::TurnCost { .. } => "turn_cost", - SseEvent::ExtensionStatus { .. } => "extension_status", - }; + let event_type = event.event_type(); Some(Ok(Event::default().event(event_type).data(data))) }); @@ -272,7 +249,7 @@ mod tests { fn test_broadcast_without_receivers() { let manager = SseManager::new(); // Should not panic even with no receivers - manager.broadcast(SseEvent::Heartbeat); + manager.broadcast(AppEvent::Heartbeat); } #[tokio::test] @@ -280,14 +257,14 @@ mod tests { let manager = SseManager::new(); let mut stream = Box::pin(manager.subscribe_raw(None).expect("should subscribe")); - manager.broadcast(SseEvent::Status { + manager.broadcast(AppEvent::Status { message: "test".to_string(), thread_id: None, }); let event = stream.next().await.unwrap(); match event { - SseEvent::Status { message, .. } => assert_eq!(message, "test"), + AppEvent::Status { message, .. } => assert_eq!(message, "test"), _ => panic!("unexpected event type"), } } @@ -299,14 +276,14 @@ mod tests { assert_eq!(manager.connection_count(), 1); - manager.broadcast(SseEvent::Thinking { + manager.broadcast(AppEvent::Thinking { message: "working".to_string(), thread_id: None, }); let event = stream.next().await.unwrap(); match event { - SseEvent::Thinking { message, .. } => assert_eq!(message, "working"), + AppEvent::Thinking { message, .. } => assert_eq!(message, "working"), _ => panic!("Expected Thinking event"), } } @@ -329,12 +306,12 @@ mod tests { let mut s2 = Box::pin(manager.subscribe_raw(None).expect("should subscribe")); assert_eq!(manager.connection_count(), 2); - manager.broadcast(SseEvent::Heartbeat); + manager.broadcast(AppEvent::Heartbeat); let e1 = s1.next().await.unwrap(); let e2 = s2.next().await.unwrap(); - assert!(matches!(e1, SseEvent::Heartbeat)); - assert!(matches!(e2, SseEvent::Heartbeat)); + assert!(matches!(e1, AppEvent::Heartbeat)); + assert!(matches!(e2, AppEvent::Heartbeat)); drop(s1); assert_eq!(manager.connection_count(), 1); @@ -373,25 +350,25 @@ mod tests { // Send event scoped to alice manager.broadcast_for_user( "alice", - SseEvent::Status { + AppEvent::Status { message: "alice only".to_string(), thread_id: None, }, ); // Send global event - manager.broadcast(SseEvent::Heartbeat); + manager.broadcast(AppEvent::Heartbeat); // Alice gets her scoped event let e = alice.next().await.unwrap(); - assert!(matches!(e, SseEvent::Status { .. })); + assert!(matches!(e, AppEvent::Status { .. })); // Alice also gets the global heartbeat let e = alice.next().await.unwrap(); - assert!(matches!(e, SseEvent::Heartbeat)); + assert!(matches!(e, AppEvent::Heartbeat)); // Bob only gets the global heartbeat (alice's event was filtered) let e = bob.next().await.unwrap(); // safety: test-only - assert!(matches!(e, SseEvent::Heartbeat)); // safety: test assertion + assert!(matches!(e, AppEvent::Heartbeat)); // safety: test assertion } } diff --git a/src/channels/web/types.rs b/src/channels/web/types.rs index 3ac4163c..fe18a824 100644 --- a/src/channels/web/types.rs +++ b/src/channels/web/types.rs @@ -114,165 +114,9 @@ pub struct ApprovalRequest { pub thread_id: Option, } -// --- SSE Event Types --- +// --- App Event (re-exported from ironclaw_common) --- -#[derive(Debug, Clone, Serialize)] -#[serde(tag = "type")] -pub enum SseEvent { - #[serde(rename = "response")] - Response { content: String, thread_id: String }, - #[serde(rename = "thinking")] - Thinking { - message: String, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - #[serde(rename = "tool_started")] - ToolStarted { - name: String, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - #[serde(rename = "tool_completed")] - ToolCompleted { - name: String, - success: bool, - #[serde(skip_serializing_if = "Option::is_none")] - error: Option, - #[serde(skip_serializing_if = "Option::is_none")] - parameters: Option, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - #[serde(rename = "tool_result")] - ToolResult { - name: String, - preview: String, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - #[serde(rename = "stream_chunk")] - StreamChunk { - content: String, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - #[serde(rename = "status")] - Status { - message: String, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - #[serde(rename = "job_started")] - JobStarted { - job_id: String, - title: String, - browse_url: String, - }, - #[serde(rename = "approval_needed")] - ApprovalNeeded { - request_id: String, - tool_name: String, - description: String, - parameters: String, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - /// Whether the "always" auto-approve option should be shown. - allow_always: bool, - }, - #[serde(rename = "auth_required")] - AuthRequired { - extension_name: String, - #[serde(skip_serializing_if = "Option::is_none")] - instructions: Option, - #[serde(skip_serializing_if = "Option::is_none")] - auth_url: Option, - #[serde(skip_serializing_if = "Option::is_none")] - setup_url: Option, - }, - #[serde(rename = "auth_completed")] - AuthCompleted { - extension_name: String, - success: bool, - message: String, - }, - #[serde(rename = "error")] - Error { - message: String, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - #[serde(rename = "heartbeat")] - Heartbeat, - - // Sandbox job streaming events (worker + Claude Code bridge) - #[serde(rename = "job_message")] - JobMessage { - job_id: String, - role: String, - content: String, - }, - #[serde(rename = "job_tool_use")] - JobToolUse { - job_id: String, - tool_name: String, - input: serde_json::Value, - }, - #[serde(rename = "job_tool_result")] - JobToolResult { - job_id: String, - tool_name: String, - output: String, - }, - #[serde(rename = "job_status")] - JobStatus { job_id: String, message: String }, - #[serde(rename = "job_result")] - JobResult { - job_id: String, - status: String, - #[serde(skip_serializing_if = "Option::is_none")] - session_id: Option, - #[serde(skip_serializing_if = "Option::is_none")] - fallback_deliverable: Option, - }, - - /// An image was generated by a tool. - #[serde(rename = "image_generated")] - ImageGenerated { - data_url: String, - #[serde(skip_serializing_if = "Option::is_none")] - path: Option, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - - /// Suggested follow-up messages for the user. - #[serde(rename = "suggestions")] - Suggestions { - suggestions: Vec, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - - /// Per-turn token usage and cost summary. - #[serde(rename = "turn_cost")] - TurnCost { - input_tokens: u64, - output_tokens: u64, - cost_usd: String, - #[serde(skip_serializing_if = "Option::is_none")] - thread_id: Option, - }, - - /// Extension activation status change (WASM channels). - #[serde(rename = "extension_status")] - ExtensionStatus { - extension_name: String, - status: String, - #[serde(skip_serializing_if = "Option::is_none")] - message: Option, - }, -} +pub use ironclaw_common::AppEvent; // --- Memory --- @@ -784,32 +628,9 @@ pub enum WsServerMessage { } impl WsServerMessage { - /// Create a WsServerMessage from an SseEvent. - pub fn from_sse_event(event: &SseEvent) -> Self { - let event_type = match event { - SseEvent::Response { .. } => "response", - SseEvent::Thinking { .. } => "thinking", - SseEvent::ToolStarted { .. } => "tool_started", - SseEvent::ToolCompleted { .. } => "tool_completed", - SseEvent::ToolResult { .. } => "tool_result", - SseEvent::StreamChunk { .. } => "stream_chunk", - SseEvent::Status { .. } => "status", - SseEvent::JobStarted { .. } => "job_started", - SseEvent::ApprovalNeeded { .. } => "approval_needed", - SseEvent::AuthRequired { .. } => "auth_required", - SseEvent::AuthCompleted { .. } => "auth_completed", - SseEvent::Error { .. } => "error", - SseEvent::Heartbeat => "heartbeat", - SseEvent::JobMessage { .. } => "job_message", - SseEvent::JobToolUse { .. } => "job_tool_use", - SseEvent::JobToolResult { .. } => "job_tool_result", - SseEvent::JobStatus { .. } => "job_status", - SseEvent::JobResult { .. } => "job_result", - SseEvent::ImageGenerated { .. } => "image_generated", - SseEvent::Suggestions { .. } => "suggestions", - SseEvent::TurnCost { .. } => "turn_cost", - SseEvent::ExtensionStatus { .. } => "extension_status", - }; + /// Create a WsServerMessage from an AppEvent. + pub fn from_app_event(event: &AppEvent) -> Self { + let event_type = event.event_type(); let data = serde_json::to_value(event).unwrap_or(serde_json::Value::Null); WsServerMessage::Event { event_type: event_type.to_string(), @@ -1101,12 +922,12 @@ mod tests { } #[test] - fn test_ws_server_from_sse_response() { - let sse = SseEvent::Response { + fn test_ws_server_from_app_event_response() { + let event = AppEvent::Response { content: "hello".to_string(), thread_id: "t1".to_string(), }; - let ws = WsServerMessage::from_sse_event(&sse); + let ws = WsServerMessage::from_app_event(&event); match ws { WsServerMessage::Event { event_type, data } => { assert_eq!(event_type, "response"); @@ -1118,12 +939,12 @@ mod tests { } #[test] - fn test_ws_server_from_sse_thinking() { - let sse = SseEvent::Thinking { + fn test_ws_server_from_app_event_thinking() { + let event = AppEvent::Thinking { message: "reasoning...".to_string(), thread_id: None, }; - let ws = WsServerMessage::from_sse_event(&sse); + let ws = WsServerMessage::from_app_event(&event); match ws { WsServerMessage::Event { event_type, data } => { assert_eq!(event_type, "thinking"); @@ -1134,8 +955,8 @@ mod tests { } #[test] - fn test_ws_server_from_sse_approval_needed() { - let sse = SseEvent::ApprovalNeeded { + fn test_ws_server_from_app_event_approval_needed() { + let event = AppEvent::ApprovalNeeded { request_id: "r1".to_string(), tool_name: "shell".to_string(), description: "Run ls".to_string(), @@ -1143,7 +964,7 @@ mod tests { thread_id: Some("t1".to_string()), allow_always: true, }; - let ws = WsServerMessage::from_sse_event(&sse); + let ws = WsServerMessage::from_app_event(&event); match ws { WsServerMessage::Event { event_type, data } => { assert_eq!(event_type, "approval_needed"); @@ -1155,9 +976,9 @@ mod tests { } #[test] - fn test_ws_server_from_sse_heartbeat() { - let sse = SseEvent::Heartbeat; - let ws = WsServerMessage::from_sse_event(&sse); + fn test_ws_server_from_app_event_heartbeat() { + let event = AppEvent::Heartbeat; + let ws = WsServerMessage::from_app_event(&event); match ws { WsServerMessage::Event { event_type, .. } => { assert_eq!(event_type, "heartbeat"); @@ -1197,8 +1018,8 @@ mod tests { } #[test] - fn test_sse_auth_required_serialize() { - let event = SseEvent::AuthRequired { + fn test_app_event_auth_required_serialize() { + let event = AppEvent::AuthRequired { extension_name: "notion".to_string(), instructions: Some("Get your token from...".to_string()), auth_url: None, @@ -1214,8 +1035,8 @@ mod tests { } #[test] - fn test_sse_auth_completed_serialize() { - let event = SseEvent::AuthCompleted { + fn test_app_event_auth_completed_serialize() { + let event = AppEvent::AuthCompleted { extension_name: "notion".to_string(), success: true, message: "notion authenticated (3 tools loaded)".to_string(), @@ -1228,14 +1049,14 @@ mod tests { } #[test] - fn test_ws_server_from_sse_auth_required() { - let sse = SseEvent::AuthRequired { + fn test_ws_server_from_app_event_auth_required() { + let event = AppEvent::AuthRequired { extension_name: "openai".to_string(), instructions: Some("Enter API key".to_string()), auth_url: None, setup_url: None, }; - let ws = WsServerMessage::from_sse_event(&sse); + let ws = WsServerMessage::from_app_event(&event); match ws { WsServerMessage::Event { event_type, data } => { assert_eq!(event_type, "auth_required"); @@ -1246,13 +1067,13 @@ mod tests { } #[test] - fn test_ws_server_from_sse_auth_completed() { - let sse = SseEvent::AuthCompleted { + fn test_ws_server_from_app_event_auth_completed() { + let event = AppEvent::AuthCompleted { extension_name: "slack".to_string(), success: false, message: "Invalid token".to_string(), }; - let ws = WsServerMessage::from_sse_event(&sse); + let ws = WsServerMessage::from_app_event(&event); match ws { WsServerMessage::Event { event_type, data } => { assert_eq!(event_type, "auth_completed"); diff --git a/src/channels/web/util.rs b/src/channels/web/util.rs index 0debe6a9..ed70c5ce 100644 --- a/src/channels/web/util.rs +++ b/src/channels/web/util.rs @@ -2,29 +2,7 @@ use crate::channels::web::types::{ToolCallInfo, TurnInfo}; -/// Truncate a string to at most `max_bytes` bytes at a char boundary, appending "...". -/// -/// If the input is wrapped in `…` and truncation -/// removes the closing tag, the tag is re-appended so downstream XML parsers -/// never see an unclosed element. -pub fn truncate_preview(s: &str, max_bytes: usize) -> String { - if s.len() <= max_bytes { - return s.to_string(); - } - // Walk backwards from max_bytes to find a valid char boundary - let mut end = max_bytes; - while end > 0 && !s.is_char_boundary(end) { - end -= 1; - } - let mut result = format!("{}...", &s[..end]); - - // Re-close if truncation cut through the closing tag. - if s.starts_with("") { - result.push_str("\n"); - } - - result -} +pub use ironclaw_common::truncate_preview; /// Build TurnInfo pairs from flat DB messages (user/tool_calls/assistant triples). /// @@ -118,88 +96,6 @@ mod tests { use super::*; use uuid::Uuid; - // ---- truncate_preview tests ---- - - #[test] - fn test_truncate_preview_short_string() { - assert_eq!(truncate_preview("hello", 10), "hello"); - } - - #[test] - fn test_truncate_preview_exact_boundary() { - assert_eq!(truncate_preview("hello", 5), "hello"); - } - - #[test] - fn test_truncate_preview_truncates_ascii() { - assert_eq!(truncate_preview("hello world", 5), "hello..."); - } - - #[test] - fn test_truncate_preview_empty_string() { - assert_eq!(truncate_preview("", 10), ""); - } - - #[test] - fn test_truncate_preview_multibyte_char_boundary() { - // '€' is 3 bytes (E2 82 AC). "a€b" = [61, E2, 82, AC, 62] = 5 bytes - // Truncating at max_bytes=3 should not split the euro sign. - let s = "a€b"; - let result = truncate_preview(s, 3); - // max_bytes=3 lands mid-€, so it walks back to byte 1 ("a") - assert_eq!(result, "a..."); - } - - #[test] - fn test_truncate_preview_emoji() { - // '🦀' is 4 bytes. "hi🦀" = 6 bytes - let s = "hi🦀"; - let result = truncate_preview(s, 4); - // max_bytes=4 lands mid-🦀, walks back to byte 2 ("hi") - assert_eq!(result, "hi..."); - } - - #[test] - fn test_truncate_preview_cjk() { - // CJK characters are 3 bytes each. "你好世界" = 12 bytes - let s = "你好世界"; - let result = truncate_preview(s, 7); - // max_bytes=7 lands mid-character (byte 7 is inside 世), walks back to 6 ("你好") - assert_eq!(result, "你好..."); - } - - #[test] - fn test_truncate_preview_zero_max_bytes() { - assert_eq!(truncate_preview("hello", 0), "..."); - } - - #[test] - fn test_truncate_preview_closes_tool_output_tag() { - let s = "\nSome very long content here\n"; - // Truncate so it cuts before the closing tag - let result = truncate_preview(s, 60); - assert!(result.ends_with("")); - assert!(result.contains("...")); - } - - #[test] - fn test_truncate_preview_no_extra_close_when_intact() { - let s = "\nshort\n"; - // The string is short enough not to be truncated - let result = truncate_preview(s, 500); - assert_eq!(result, s); - // Should not have a duplicate closing tag - assert_eq!(result.matches("").count(), 1); - } - - #[test] - fn test_truncate_preview_non_xml_unaffected() { - let s = "Just a plain long string that gets truncated"; - let result = truncate_preview(s, 10); - assert_eq!(result, "Just a pla..."); - assert!(!result.contains("")); - } - // ---- build_turns_from_db_messages tests ---- fn make_msg(role: &str, content: &str, offset_ms: i64) -> crate::history::ConversationMessage { diff --git a/src/channels/web/ws.rs b/src/channels/web/ws.rs index 9d4e919c..51beaafd 100644 --- a/src/channels/web/ws.rs +++ b/src/channels/web/ws.rs @@ -97,7 +97,7 @@ pub async fn handle_ws_connection( let msg = tokio::select! { event = event_stream.next() => { match event { - Some(sse_event) => WsServerMessage::from_sse_event(&sse_event), + Some(app_event) => WsServerMessage::from_app_event(&app_event), None => break, // Broadcast channel closed } } @@ -275,7 +275,7 @@ async fn handle_client_message( if result.verification.is_some() { state.sse.broadcast_for_user( user_id, - crate::channels::web::types::SseEvent::AuthRequired { + crate::channels::web::types::AppEvent::AuthRequired { extension_name: extension_name.clone(), instructions: Some(result.message), auth_url: None, @@ -286,7 +286,7 @@ async fn handle_client_message( crate::channels::web::server::clear_auth_mode(state, user_id).await; state.sse.broadcast_for_user( user_id, - crate::channels::web::types::SseEvent::AuthCompleted { + crate::channels::web::types::AppEvent::AuthCompleted { extension_name, success: true, message: result.message, @@ -299,7 +299,7 @@ async fn handle_client_message( if matches!(e, crate::extensions::ExtensionError::ValidationFailed(_)) { state.sse.broadcast_for_user( user_id, - crate::channels::web::types::SseEvent::AuthRequired { + crate::channels::web::types::AppEvent::AuthRequired { extension_name: extension_name.clone(), instructions: Some(msg.clone()), auth_url: None, diff --git a/src/extensions/manager.rs b/src/extensions/manager.rs index 0f308352..90920767 100644 --- a/src/extensions/manager.rs +++ b/src/extensions/manager.rs @@ -1118,7 +1118,7 @@ impl ExtensionManager { /// Broadcast an extension status change to the web UI via SSE. async fn broadcast_extension_status(&self, name: &str, status: &str, message: Option<&str>) { if let Some(ref sse) = *self.sse_manager.read().await { - sse.broadcast(crate::channels::web::types::SseEvent::ExtensionStatus { + sse.broadcast(ironclaw_common::AppEvent::ExtensionStatus { extension_name: name.to_string(), status: status.to_string(), message: message.map(|m| m.to_string()), @@ -3288,7 +3288,7 @@ impl ExtensionManager { } .await; - // Broadcast SSE event + // Broadcast auth result event let (success, message) = match result { Ok(()) => (true, format!("{} authenticated successfully", display_name)), Err(ref e) => ( @@ -3314,7 +3314,7 @@ impl ExtensionManager { } if let Some(ref sse) = sse_manager { - sse.broadcast(crate::channels::web::types::SseEvent::AuthCompleted { + sse.broadcast(ironclaw_common::AppEvent::AuthCompleted { extension_name: ext_name, success, message, diff --git a/src/orchestrator/api.rs b/src/orchestrator/api.rs index 00f8a4da..37085a8b 100644 --- a/src/orchestrator/api.rs +++ b/src/orchestrator/api.rs @@ -14,7 +14,6 @@ use serde::{Deserialize, Serialize}; use tokio::sync::{Mutex, broadcast}; use uuid::Uuid; -use crate::channels::web::types::SseEvent; use crate::db::Database; use crate::llm::{CompletionRequest, LlmProvider, ToolCompletionRequest}; use crate::orchestrator::auth::{TokenStore, worker_auth_middleware}; @@ -25,6 +24,7 @@ use crate::worker::api::{ CompletionReport, CredentialResponse, JobDescription, ProxyCompletionRequest, ProxyCompletionResponse, ProxyToolCompletionRequest, ProxyToolCompletionResponse, StatusUpdate, }; +use ironclaw_common::AppEvent; /// A follow-up prompt queued for a Claude Code bridge. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -41,7 +41,7 @@ pub struct OrchestratorState { pub token_store: TokenStore, /// Broadcast channel for job events (consumed by the web gateway SSE). /// Tuple: (job_id, user_id, event). - pub job_event_tx: Option>, + pub job_event_tx: Option>, /// Buffered follow-up prompts for sandbox jobs, keyed by job_id. pub prompt_queue: Arc>>>, /// Database handle for persisting job events. @@ -277,10 +277,10 @@ async fn job_event_handler( }); } - // Convert to SSE event and broadcast + // Convert to app event and broadcast let job_id_str = job_id.to_string(); - let sse_event = match payload.event_type.as_str() { - "message" => SseEvent::JobMessage { + let app_event = match payload.event_type.as_str() { + "message" => AppEvent::JobMessage { job_id: job_id_str, role: payload .data @@ -295,7 +295,7 @@ async fn job_event_handler( .unwrap_or("") .to_string(), }, - "tool_use" => SseEvent::JobToolUse { + "tool_use" => AppEvent::JobToolUse { job_id: job_id_str, tool_name: payload .data @@ -309,7 +309,7 @@ async fn job_event_handler( .cloned() .unwrap_or(serde_json::Value::Null), }, - "tool_result" => SseEvent::JobToolResult { + "tool_result" => AppEvent::JobToolResult { job_id: job_id_str, tool_name: payload .data @@ -324,7 +324,7 @@ async fn job_event_handler( .unwrap_or("") .to_string(), }, - "result" => SseEvent::JobResult { + "result" => AppEvent::JobResult { job_id: job_id_str, status: payload .data @@ -344,7 +344,7 @@ async fn job_event_handler( // gain context/memory tracking capabilities. fallback_deliverable: payload.data.get("fallback_deliverable").cloned(), }, - _ => SseEvent::JobStatus { + _ => AppEvent::JobStatus { job_id: job_id_str, message: payload .data @@ -390,9 +390,9 @@ async fn job_event_handler( }; if user_id.is_empty() { - let _ = tx.send((job_id, String::new(), sse_event)); + let _ = tx.send((job_id, String::new(), app_event)); } else { - let _ = tx.send((job_id, user_id, sse_event)); + let _ = tx.send((job_id, user_id, app_event)); } } @@ -817,7 +817,7 @@ mod tests { // No store configured, so user_id falls back to empty string. assert_eq!(recv_uid, ""); match event { - SseEvent::JobMessage { + AppEvent::JobMessage { job_id: jid, role, content, @@ -872,7 +872,7 @@ mod tests { let (_recv_id, _recv_uid, event) = rx.recv().await.unwrap(); match event { - SseEvent::JobToolUse { tool_name, .. } => { + AppEvent::JobToolUse { tool_name, .. } => { assert_eq!(tool_name, "shell"); } other => panic!("Expected JobToolUse, got {:?}", other), @@ -918,7 +918,7 @@ mod tests { let (_recv_id, _recv_uid, event) = rx.recv().await.unwrap(); // Unknown event types fall through to JobStatus - assert!(matches!(event, SseEvent::JobStatus { .. })); + assert!(matches!(event, AppEvent::JobStatus { .. })); } // -- Status update test -- diff --git a/src/orchestrator/mod.rs b/src/orchestrator/mod.rs index 896b5648..8d09dc53 100644 --- a/src/orchestrator/mod.rs +++ b/src/orchestrator/mod.rs @@ -46,10 +46,10 @@ use std::sync::Arc; use tokio::sync::{Mutex, broadcast}; use uuid::Uuid; -use crate::channels::web::types::SseEvent; use crate::db::Database; use crate::llm::LlmProvider; use crate::secrets::SecretsStore; +use ironclaw_common::AppEvent; /// Resolve the orchestrator port from the `ORCHESTRATOR_PORT` environment /// variable, falling back to 50051. @@ -63,7 +63,7 @@ fn resolve_orchestrator_port() -> u16 { /// Result of orchestrator setup, containing all handles needed by the agent. pub struct OrchestratorSetup { pub container_job_manager: Option>, - pub job_event_tx: Option>, + pub job_event_tx: Option>, pub prompt_queue: Arc>>>, pub docker_status: crate::sandbox::DockerStatus, } diff --git a/src/tools/builtin/job.rs b/src/tools/builtin/job.rs index 86d7e44d..4c711e69 100644 --- a/src/tools/builtin/job.rs +++ b/src/tools/builtin/job.rs @@ -17,7 +17,6 @@ use uuid::Uuid; use crate::bootstrap::ironclaw_base_dir; use crate::channels::IncomingMessage; -use crate::channels::web::types::SseEvent; use crate::context::{ContextManager, JobContext, JobState}; use crate::db::Database; use crate::history::SandboxJobRecord; @@ -25,6 +24,7 @@ use crate::orchestrator::auth::CredentialGrant; use crate::orchestrator::job_manager::{ContainerJobManager, JobMode}; use crate::secrets::SecretsStore; use crate::tools::tool::{ApprovalRequirement, Tool, ToolError, ToolOutput, require_str}; +use ironclaw_common::AppEvent; /// Lazy scheduler reference, filled after Agent::new creates the Scheduler. /// @@ -85,7 +85,7 @@ pub struct CreateJobTool { job_manager: Option>, store: Option>, /// Broadcast sender for job events (used to subscribe a monitor). - event_tx: Option>, + event_tx: Option>, /// Injection channel for pushing messages into the agent loop. inject_tx: Option>, /// Encrypted secrets store for validating credential grants. @@ -120,7 +120,7 @@ impl CreateJobTool { /// monitor that forwards Claude Code output to the main agent loop. pub fn with_monitor_deps( mut self, - event_tx: tokio::sync::broadcast::Sender<(Uuid, String, SseEvent)>, + event_tx: tokio::sync::broadcast::Sender<(Uuid, String, AppEvent)>, inject_tx: tokio::sync::mpsc::Sender, ) -> Self { self.event_tx = Some(event_tx); diff --git a/src/tools/registry.rs b/src/tools/registry.rs index bc3be144..8c08633b 100644 --- a/src/tools/registry.rs +++ b/src/tools/registry.rs @@ -383,11 +383,7 @@ impl ToolRegistry { job_manager: Option>, store: Option>, job_event_tx: Option< - tokio::sync::broadcast::Sender<( - uuid::Uuid, - String, - crate::channels::web::types::SseEvent, - )>, + tokio::sync::broadcast::Sender<(uuid::Uuid, String, ironclaw_common::AppEvent)>, >, inject_tx: Option>, prompt_queue: Option, diff --git a/src/worker/job.rs b/src/worker/job.rs index b2e3f7e6..ed261039 100644 --- a/src/worker/job.rs +++ b/src/worker/job.rs @@ -18,7 +18,6 @@ use crate::agent::agentic_loop::{ }; use crate::agent::scheduler::WorkerMessage; use crate::agent::task::TaskOutput; -use crate::channels::web::types::SseEvent; use crate::context::{ContextManager, JobState}; use crate::db::Database; use crate::error::Error; @@ -33,6 +32,7 @@ use crate::tools::rate_limiter::RateLimitResult; use crate::tools::{ ApprovalContext, ToolRegistry, autonomous_unavailable_error, prepare_tool_params, redact_params, }; +use ironclaw_common::AppEvent; /// Shared dependencies for worker execution. /// @@ -48,7 +48,7 @@ pub struct WorkerDeps { pub hooks: Arc, pub timeout: Duration, pub use_planning: bool, - /// SSE manager for live job event streaming to the web gateway. + /// Broadcast sender for live job event streaming to the web gateway. pub sse_tx: Option>, /// Approval context for tool execution. When `None`, all non-`Never` tools are /// blocked (legacy behavior). When `Some`, the context determines which tools @@ -141,7 +141,7 @@ impl Worker { if let Some(ref sse) = self.deps.sse_tx { let job_id_str = job_id.to_string(); let event = match event_type { - "message" => Some(SseEvent::JobMessage { + "message" => Some(AppEvent::JobMessage { job_id: job_id_str, role: data .get("role") @@ -154,7 +154,7 @@ impl Worker { .unwrap_or("") .to_string(), }), - "tool_use" => Some(SseEvent::JobToolUse { + "tool_use" => Some(AppEvent::JobToolUse { job_id: job_id_str, tool_name: data .get("tool_name") @@ -166,7 +166,7 @@ impl Worker { .cloned() .unwrap_or(serde_json::Value::Null), }), - "tool_result" => Some(SseEvent::JobToolResult { + "tool_result" => Some(AppEvent::JobToolResult { job_id: job_id_str, tool_name: data .get("tool_name") @@ -179,7 +179,7 @@ impl Worker { .unwrap_or("") .to_string(), }), - "status" => Some(SseEvent::JobStatus { + "status" => Some(AppEvent::JobStatus { job_id: job_id_str, message: data .get("message") @@ -187,7 +187,7 @@ impl Worker { .unwrap_or("") .to_string(), }), - "result" => Some(SseEvent::JobResult { + "result" => Some(AppEvent::JobResult { job_id: job_id_str, status: data .get("status") diff --git a/tests/multi_tenant_integration.rs b/tests/multi_tenant_integration.rs index f2529866..227fa721 100644 --- a/tests/multi_tenant_integration.rs +++ b/tests/multi_tenant_integration.rs @@ -307,7 +307,7 @@ fn per_user_rate_limiter_single_user_mode() { #[tokio::test] async fn sse_scoped_event_only_delivered_to_target_user() { - use ironclaw::channels::web::types::SseEvent; + use ironclaw_common::AppEvent; use tokio_stream::StreamExt; let manager = SseManager::new(); @@ -325,34 +325,34 @@ async fn sse_scoped_event_only_delivered_to_target_user() { // Send event scoped to alice manager.broadcast_for_user( ALICE_USER_ID, - SseEvent::Status { + AppEvent::Status { message: "alice's event".to_string(), thread_id: None, }, ); // Send global heartbeat (both should get it) - manager.broadcast(SseEvent::Heartbeat); + manager.broadcast(AppEvent::Heartbeat); // Alice gets her scoped event first let e = alice_stream.next().await.unwrap(); match &e { - SseEvent::Status { message, .. } => assert_eq!(message, "alice's event"), + AppEvent::Status { message, .. } => assert_eq!(message, "alice's event"), _ => panic!("Expected Status, got {:?}", e), } // Alice also gets heartbeat let e = alice_stream.next().await.unwrap(); - assert!(matches!(e, SseEvent::Heartbeat)); + assert!(matches!(e, AppEvent::Heartbeat)); // Bob only gets the heartbeat (alice's event was filtered) let e = bob_stream.next().await.unwrap(); - assert!(matches!(e, SseEvent::Heartbeat)); + assert!(matches!(e, AppEvent::Heartbeat)); } #[tokio::test] async fn sse_global_event_delivered_to_all_users() { - use ironclaw::channels::web::types::SseEvent; + use ironclaw_common::AppEvent; use tokio_stream::StreamExt; let manager = SseManager::new(); @@ -367,7 +367,7 @@ async fn sse_global_event_delivered_to_all_users() { .expect("subscribe"), ); - manager.broadcast(SseEvent::Status { + manager.broadcast(AppEvent::Status { message: "global announcement".to_string(), thread_id: None, }); @@ -375,7 +375,7 @@ async fn sse_global_event_delivered_to_all_users() { let ea = alice.next().await.unwrap(); let eb = bob.next().await.unwrap(); match (&ea, &eb) { - (SseEvent::Status { message: a, .. }, SseEvent::Status { message: b, .. }) => { + (AppEvent::Status { message: a, .. }, AppEvent::Status { message: b, .. }) => { assert_eq!(a, "global announcement"); assert_eq!(b, "global announcement"); } @@ -385,7 +385,7 @@ async fn sse_global_event_delivered_to_all_users() { #[tokio::test] async fn sse_user_b_event_not_visible_to_user_a() { - use ironclaw::channels::web::types::SseEvent; + use ironclaw_common::AppEvent; use tokio_stream::StreamExt; let manager = SseManager::new(); @@ -398,19 +398,19 @@ async fn sse_user_b_event_not_visible_to_user_a() { // Send event for bob only manager.broadcast_for_user( BOB_USER_ID, - SseEvent::Response { + AppEvent::Response { content: "bob's secret".to_string(), thread_id: "t1".to_string(), }, ); // Send heartbeat so alice has something to receive - manager.broadcast(SseEvent::Heartbeat); + manager.broadcast(AppEvent::Heartbeat); // Alice should only get heartbeat, not bob's response let e = alice.next().await.unwrap(); assert!( - matches!(e, SseEvent::Heartbeat), + matches!(e, AppEvent::Heartbeat), "Expected Heartbeat, got {:?}", e ); @@ -418,7 +418,7 @@ async fn sse_user_b_event_not_visible_to_user_a() { #[tokio::test] async fn sse_unscoped_subscriber_receives_all_events() { - use ironclaw::channels::web::types::SseEvent; + use ironclaw_common::AppEvent; use tokio_stream::StreamExt; let manager = SseManager::new(); @@ -427,19 +427,19 @@ async fn sse_unscoped_subscriber_receives_all_events() { manager.broadcast_for_user( ALICE_USER_ID, - SseEvent::Status { + AppEvent::Status { message: "alice only".to_string(), thread_id: None, }, ); manager.broadcast_for_user( BOB_USER_ID, - SseEvent::Status { + AppEvent::Status { message: "bob only".to_string(), thread_id: None, }, ); - manager.broadcast(SseEvent::Heartbeat); + manager.broadcast(AppEvent::Heartbeat); // Unscoped subscriber gets ALL three events let e1 = stream.next().await.unwrap(); @@ -447,14 +447,14 @@ async fn sse_unscoped_subscriber_receives_all_events() { let e3 = stream.next().await.unwrap(); match &e1 { - SseEvent::Status { message, .. } => assert_eq!(message, "alice only"), + AppEvent::Status { message, .. } => assert_eq!(message, "alice only"), _ => panic!("Expected alice's Status"), } match &e2 { - SseEvent::Status { message, .. } => assert_eq!(message, "bob only"), + AppEvent::Status { message, .. } => assert_eq!(message, "bob only"), _ => panic!("Expected bob's Status"), } - assert!(matches!(e3, SseEvent::Heartbeat)); + assert!(matches!(e3, AppEvent::Heartbeat)); } // =========================================================================== @@ -881,7 +881,7 @@ async fn full_server_jobs_endpoint_rejected_without_auth() { #[tokio::test] async fn full_server_ws_multi_user_event_isolation() { use futures::StreamExt; - use ironclaw::channels::web::types::SseEvent; + use ironclaw_common::AppEvent; use tokio_tungstenite::tungstenite::Message; use tokio_tungstenite::tungstenite::client::IntoClientRequest; @@ -914,14 +914,14 @@ async fn full_server_ws_multi_user_event_isolation() { // Broadcast an event scoped to Alice only state.sse.broadcast_for_user( ALICE_USER_ID, - SseEvent::Status { + AppEvent::Status { message: "alice-only-event".to_string(), thread_id: None, }, ); // Broadcast a global heartbeat so Bob has something to receive - state.sse.broadcast(SseEvent::Heartbeat); + state.sse.broadcast(AppEvent::Heartbeat); // Alice should get her scoped event let alice_msg = tokio::time::timeout(Duration::from_secs(2), alice_ws.next()) diff --git a/tests/ws_gateway_integration.rs b/tests/ws_gateway_integration.rs index a6db5af7..0ec5c929 100644 --- a/tests/ws_gateway_integration.rs +++ b/tests/ws_gateway_integration.rs @@ -5,7 +5,7 @@ //! - WebSocket upgrade with auth //! - Ping/pong //! - Client message → agent msg_tx -//! - Broadcast SSE event → WebSocket client +//! - Broadcast AppEvent → WebSocket client //! - Connection tracking (counter increment/decrement) //! - Gateway status endpoint @@ -22,8 +22,8 @@ use tokio_tungstenite::tungstenite::client::IntoClientRequest; use ironclaw::channels::IncomingMessage; use ironclaw::channels::web::server::{GatewayState, start_server}; use ironclaw::channels::web::sse::SseManager; -use ironclaw::channels::web::types::SseEvent; use ironclaw::channels::web::ws::WsConnectionTracker; +use ironclaw_common::AppEvent; const AUTH_TOKEN: &str = "test-token-12345"; const TIMEOUT: Duration = Duration::from_secs(5); @@ -164,8 +164,8 @@ async fn test_ws_broadcast_event_received() { // Give the connection a moment to fully establish tokio::time::sleep(Duration::from_millis(50)).await; - // Broadcast an SSE event (simulates agent sending a response) - state.sse.broadcast(SseEvent::Response { + // Broadcast an event (simulates agent sending a response) + state.sse.broadcast(AppEvent::Response { content: "agent says hi".to_string(), thread_id: "t1".to_string(), }); @@ -186,7 +186,7 @@ async fn test_ws_thinking_event() { let mut ws = connect_ws(addr).await; tokio::time::sleep(Duration::from_millis(50)).await; - state.sse.broadcast(SseEvent::Thinking { + state.sse.broadcast(AppEvent::Thinking { message: "analyzing...".to_string(), thread_id: None, }); @@ -311,22 +311,22 @@ async fn test_ws_multiple_events_in_sequence() { tokio::time::sleep(Duration::from_millis(50)).await; // Broadcast multiple events rapidly - state.sse.broadcast(SseEvent::Thinking { + state.sse.broadcast(AppEvent::Thinking { message: "step 1".to_string(), thread_id: None, }); - state.sse.broadcast(SseEvent::ToolStarted { + state.sse.broadcast(AppEvent::ToolStarted { name: "shell".to_string(), thread_id: None, }); - state.sse.broadcast(SseEvent::ToolCompleted { + state.sse.broadcast(AppEvent::ToolCompleted { name: "shell".to_string(), success: true, error: None, parameters: None, thread_id: None, }); - state.sse.broadcast(SseEvent::Response { + state.sse.broadcast(AppEvent::Response { content: "done".to_string(), thread_id: "t1".to_string(), }); From 6daa2f155f2683cf93669cac5844b6d85400b7a5 Mon Sep 17 00:00:00 2001 From: Jacob Lasky Date: Wed, 25 Mar 2026 03:31:44 -0400 Subject: [PATCH 3/8] fix: ensure LLM calls always end with user message (closes #763) (#1259) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: ensure LLM calls always end with user message (closes #763) Claude 4.6 models (claude-sonnet-4-6, claude-opus-4-6) no longer support assistant message prefill — any LLM call where the conversation ends on an assistant message is rejected with HTTP 400 "This model does not support assistant message prefill". The same root cause also triggers NEAR AI's "No user query found in messages" 400 error for the routine engine path. Two fixes: 1. src/worker/container.rs — before_llm_call() After poll_and_inject_prompt(), if no user follow-up arrived and handle_text_response() left an assistant message at the end of the conversation, inject a sentinel "Continue." user message before the next LLM call. 2. src/agent/routine_engine.rs — execute_lightweight_with_tools() Before the force_text final completion call, ensure messages end with a user-role message. Tool result messages (Role::Tool) satisfy Anthropic but not NEAR AI; assistant messages satisfy neither. Also updates the worker system prompt to instruct the agent to include the phrase "The job is complete" in its final message, so the agentic loop can detect termination reliably. Tested with claude-sonnet-4-6 and claude-opus-4-6. Workaround: ANTHROPIC_MODEL=claude-sonnet-4-20250514 (still supports prefill). * fix: broaden sentinel guard to any non-user message (per review) Gemini suggested the Role::Assistant check in before_llm_call() is too specific. Changed to !Role::User to match the routine_engine.rs fix and cover tool results too. * fix: address zmanian review — JobDelegate sentinel, shared helper, NearAI complete() flattening - Extract ensure_ends_with_user_message() to src/util.rs with 4 unit tests (empty list, after assistant, after tool result, no-op when already user) - Add sentinel guard to JobDelegate::before_llm_call() in src/worker/job.rs so scheduler jobs (CreateJob / /job path) no longer hit Claude 4.6 / NEAR AI 400s - Replace inline guards in ContainerDelegate and routine_engine.rs with the shared helper — all 3 call sites now use one implementation - Fix complete() in nearai_chat.rs to apply flatten_tool_messages when flatten_tool_messages=true — previously only complete_with_tools() flattened, so force_text paths could still send role:"tool" messages to NEAR AI - Update stale comment in container.rs: "assistant message" → "non-user message" - Add flatten tests in nearai_chat.rs covering the complete() path Co-Authored-By: Claude Sonnet 4.6 * ci: fix fmt and tar advisory --------- Co-authored-by: Jacob Lasky Co-authored-by: Claude Sonnet 4.6 Co-authored-by: Illia Polosukhin Co-authored-by: firat.sertgoz --- src/agent/routine_engine.rs | 5 ++- src/llm/nearai_chat.rs | 70 +++++++++++++++++++++++++++++++++++-- src/util.rs | 52 ++++++++++++++++++++++++++- src/worker/container.rs | 6 +++- src/worker/job.rs | 5 +++ 5 files changed, 133 insertions(+), 5 deletions(-) diff --git a/src/agent/routine_engine.rs b/src/agent/routine_engine.rs index 39acb83d..9c55903f 100644 --- a/src/agent/routine_engine.rs +++ b/src/agent/routine_engine.rs @@ -1541,7 +1541,10 @@ async fn execute_lightweight_with_tools( let force_text = iteration >= max_iterations; if force_text { - // Final iteration: no tools, just get text response + // Final iteration: no tools, just get text response. + // Claude 4.6 rejects assistant prefill; NEAR AI rejects any non-user-ending + // conversation. Ensure the last message is user-role. + crate::util::ensure_ends_with_user_message(&mut messages); let request = CompletionRequest::new(messages) .with_max_tokens(effective_max_tokens) .with_temperature(0.3); diff --git a/src/llm/nearai_chat.rs b/src/llm/nearai_chat.rs index acbff6ad..5372d76d 100644 --- a/src/llm/nearai_chat.rs +++ b/src/llm/nearai_chat.rs @@ -463,8 +463,15 @@ impl LlmProvider for NearAiChatProvider { let model = req.model.unwrap_or_else(|| self.active_model_name()); let mut raw_messages = req.messages; crate::llm::provider::sanitize_tool_messages(&mut raw_messages); - let messages: Vec = - raw_messages.into_iter().map(|m| m.into()).collect(); + let raw: Vec = raw_messages.into_iter().map(|m| m.into()).collect(); + + // NEAR AI rejects `role:"tool"` messages even on text-only completion paths. + // Apply the same flattening used by complete_with_tools(). + let messages = if self.flatten_tool_messages { + flatten_tool_messages(raw) + } else { + raw + }; let request = ChatCompletionRequest { model, @@ -2193,6 +2200,65 @@ mod tests { assert_eq!(deserialized.function.arguments, r#"{"city":"London"}"#); } + // -- flatten_tool_messages in complete() path ---------------------------- + + #[test] + fn test_flatten_applied_on_text_only_path() { + // Verify that flatten_tool_messages converts tool-role messages to user + // messages (mirrors the complete_with_tools path). + let messages = vec![ + ChatCompletionMessage { + role: "user".to_string(), + content: Some(MessageContent::Text("run it".to_string())), + tool_call_id: None, + name: None, + tool_calls: None, + }, + ChatCompletionMessage { + role: "tool".to_string(), + content: Some(MessageContent::Text("ok".to_string())), + tool_call_id: Some("call_1".to_string()), + name: Some("run_cmd".to_string()), + tool_calls: None, + }, + ]; + let flattened = flatten_tool_messages(messages); + assert_eq!(flattened.len(), 2); + assert_eq!(flattened[1].role, "user"); + let text = flattened[1] + .content + .as_ref() + .and_then(|c| c.as_text()) + .unwrap(); + assert!(text.contains("run_cmd"), "should reference tool name"); + assert!(text.contains("ok"), "should include tool result"); + } + + #[test] + fn test_no_flatten_when_no_tool_messages() { + // When there are no tool-role messages, flatten_tool_messages is a no-op. + let messages = vec![ + ChatCompletionMessage { + role: "user".to_string(), + content: Some(MessageContent::Text("hi".to_string())), + tool_call_id: None, + name: None, + tool_calls: None, + }, + ChatCompletionMessage { + role: "assistant".to_string(), + content: Some(MessageContent::Text("hello".to_string())), + tool_call_id: None, + name: None, + tool_calls: None, + }, + ]; + let result = flatten_tool_messages(messages); + // No tool messages → unchanged roles + assert_eq!(result[0].role, "user"); + assert_eq!(result[1].role, "assistant"); + } + // -- api_url edge cases --------------------------------------------------- #[test] diff --git a/src/util.rs b/src/util.rs index 866f623c..a76f3b27 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,5 +1,7 @@ //! Shared utility functions used across the codebase. +use crate::llm::{ChatMessage, Role}; + /// Find the largest valid UTF-8 char boundary at or before `pos`. /// /// Polyfill for `str::floor_char_boundary` (nightly-only). Use when @@ -16,6 +18,17 @@ pub fn floor_char_boundary(s: &str, pos: usize) -> usize { i } +/// Ensure the last message in `messages` is a user-role message. +/// +/// NEAR AI rejects conversations that don't end with a user message; +/// Claude 4.6 rejects assistant prefill. Call this before any LLM +/// completion request to satisfy both requirements. +pub fn ensure_ends_with_user_message(messages: &mut Vec) { + if !matches!(messages.last(), Some(m) if m.role == Role::User) { + messages.push(ChatMessage::user("Continue.")); + } +} + /// Check if an LLM response explicitly signals that a job/task is complete. /// /// Uses phrase-level matching to avoid false positives from bare words like @@ -72,7 +85,8 @@ pub fn llm_signals_completion(response: &str) -> bool { #[cfg(test)] mod tests { - use crate::util::{floor_char_boundary, llm_signals_completion}; + use crate::llm::ChatMessage; + use crate::util::{ensure_ends_with_user_message, floor_char_boundary, llm_signals_completion}; // ── floor_char_boundary ── @@ -103,6 +117,42 @@ mod tests { assert_eq!(floor_char_boundary("", 5), 0); } + // ── ensure_ends_with_user_message ── + + #[test] + fn ensure_user_message_injects_when_empty() { + let mut msgs: Vec = vec![]; + ensure_ends_with_user_message(&mut msgs); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0].role, crate::llm::Role::User); + } + + #[test] + fn ensure_user_message_injects_after_assistant() { + let mut msgs = vec![ChatMessage::user("hi"), ChatMessage::assistant("hello")]; + ensure_ends_with_user_message(&mut msgs); + assert_eq!(msgs.len(), 3); + assert_eq!(msgs[2].role, crate::llm::Role::User); + } + + #[test] + fn ensure_user_message_injects_after_tool_result() { + let mut msgs = vec![ + ChatMessage::user("run tool"), + ChatMessage::tool_result("call_1", "my_tool", "result"), + ]; + ensure_ends_with_user_message(&mut msgs); + assert_eq!(msgs.len(), 3); + assert_eq!(msgs[2].role, crate::llm::Role::User); + } + + #[test] + fn ensure_user_message_no_op_when_already_user() { + let mut msgs = vec![ChatMessage::user("hello")]; + ensure_ends_with_user_message(&mut msgs); + assert_eq!(msgs.len(), 1); + } + // ── llm_signals_completion ── #[test] diff --git a/src/worker/container.rs b/src/worker/container.rs index e0933975..5d8e03b5 100644 --- a/src/worker/container.rs +++ b/src/worker/container.rs @@ -151,7 +151,7 @@ Job: {} Description: {} You have tools for shell commands, file operations, and code editing. -Work independently to complete this job. Report when done."#, +Work independently to complete this job. When finished, your final message MUST include the phrase "The job is complete" to signal termination."#, job.title, job.description ))); @@ -373,6 +373,10 @@ impl LoopDelegate for ContainerDelegate { // Poll for follow-up prompts from the user self.poll_and_inject_prompt(reason_ctx).await; + // Claude 4.6 rejects assistant prefill; NEAR AI rejects any non-user-ending + // conversation. Ensure the last message is user-role before calling the LLM. + crate::util::ensure_ends_with_user_message(&mut reason_ctx.messages); + // Refresh tools (in case WASM tools were built) reason_ctx.available_tools = self.tools.tool_definitions().await; diff --git a/src/worker/job.rs b/src/worker/job.rs index ed261039..9d5794ca 100644 --- a/src/worker/job.rs +++ b/src/worker/job.rs @@ -1232,6 +1232,11 @@ impl<'a> LoopDelegate for JobDelegate<'a> { ) -> Option { // Refresh tool definitions so newly built tools become visible reason_ctx.available_tools = self.worker.tools().tool_definitions().await; + + // Claude 4.6 rejects assistant prefill; NEAR AI rejects any non-user-ending + // conversation. Ensure the last message is user-role before calling the LLM. + crate::util::ensure_ends_with_user_message(&mut reason_ctx.messages); + None } From 41ed0a0f9814d754c17df80c14d263ae10e09b45 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Wed, 25 Mar 2026 08:35:41 -0700 Subject: [PATCH 4/8] feat(agent): thread per-tool reasoning through provider, session, and all surfaces (#1513) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(agent): thread per-tool reasoning from LLM through to REPL, HTTP, SSE, and DB Add end-to-end agent reasoning summaries so users can see *why* the agent chose specific tools, not just what it did. - Add `reasoning: Option` to `ToolCall` (all providers) - Populate from LLM response content in `Reasoning::respond_with_tools` and `select_tools`, with per-tool override when providers supply it - Extend `Turn` with `narrative` and `TurnToolCall` with `rationale` + `tool_call_id` for identity-based result matching - Persist reasoning in DB via existing tool_calls JSON (no migration) - Add `StatusUpdate::ReasoningUpdate` and `SseEvent::ReasoningUpdate` + `SseEvent::JobReasoning` for real-time streaming - Emit reasoning events in both chat dispatcher and worker job path - Add `/reasoning [N|all]` command for inspecting turn reasoning - Surface `narrative` and `rationale` in HTTP `/api/chat/history` Based on the design from #361 and #456, reconstructed cleanly with Option to minimize blast radius (vs mandatory String that broke compilation in #456). Closes #456 Co-Authored-By: panosAthDBX <47406510+panosAthDBX@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) * fix: address PR review feedback from Gemini and Copilot - Fix `_ => Ok(None)` in agent_loop.rs to avoid accidental shutdown - Fix fallback in record_tool_result_for/record_tool_error_for to use first pending call instead of last_mut (parallel execution safety) - Include per-tool decisions in WASM channel reasoning messages - Apply truncate_at_tool_tags + clean_response to shared_reasoning in select_tools (parity with respond_with_tools) - Persist turn-level narrative to DB in tool_calls JSON wrapper - Parse both old (array) and new (object) tool_calls formats in build_turns_from_db_messages for backward compatibility - Populate reasoning from action.reasoning in execute_plan ToolCalls [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) * fix: address second round of review comments + merge fixes - Add reasoning: None to new github_copilot.rs ToolCall sites (from staging merge) - Run cargo fmt on 4 files with formatting diffs - Truncate narrative to 1000 chars before DB persistence - Clone turn data and drop session lock in /reasoning command - Extract ToolDecisionDto::from_json_array shared helper (deduplicate worker/job.rs and orchestrator/api.rs) - Add unit tests for wrapped tool_calls JSON format with narrative [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) * fix: address third round of review comments (Copilot + serrrfirat) - Reword ToolCall.reasoning docstring to reflect provider-supplied or fallback contract - Sanitize narrative through SafetyLayer before storage/emission - Clean per-tool reasoning via truncate_at_tool_tags + clean_response in select_tools (parity with shared reasoning) - Convert 4 approval-path recording sites in thread_ops.rs to identity-based record_tool_result_for/record_tool_error_for - Preserve tool_call_id and reasoning through restore_from_messages - Fix has_result/has_error to reject JSON null values - Truncate tool_call_id to 128 chars before DB persistence - Add 4 unit tests for record_tool_result_for/error_for edge cases Co-Authored-By: Claude Opus 4.6 (1M context) * fix: address zmanian review — sanitize JobDelegate reasoning + warn on dropped results - Sanitize narrative and per-tool rationale through SafetyLayer in JobDelegate reasoning events (parity with ChatDelegate) - Add tracing::warn when record_tool_result_for/error_for drops a result because no matching or pending tool call exists - Add 3 unit tests for reasoning normalization (thinking tags, tool tags, empty-after-cleaning) Co-Authored-By: Claude Opus 4.6 (1M context) * fix: address 4 remaining unreplied review comments - Clean per-tool reasoning in respond_with_tools via truncate_at_tool_tags + clean_response (parity with select_tools) - Handle wrapped JSON format in rebuild_chat_messages_from_db so cold hydration works after persist_tool_calls format change - Update persist_tool_calls doc comment to describe new JSON shape - Sanitize per-tool rationale through SafetyLayer in ChatDelegate before emission and storage (parity with JobDelegate) Co-Authored-By: Claude Opus 4.6 (1M context) * fix: address zmanian review round 2 - Add tracing::debug on fallback-to-pending path in record_tool_result_for and record_tool_error_for (item 1) - Add comment explaining why /reasoning is special-cased in agent_loop.rs (item 4) - Items 2 (narrative persistence), 3 (rationale sanitization), and 5 (catch-all fix) were already addressed in prior commits Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: panosAthDBX <47406510+panosAthDBX@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) --- crates/ironclaw_common/src/event.rs | 55 ++++++++ crates/ironclaw_common/src/lib.rs | 2 +- src/agent/agent_loop.rs | 16 +++ src/agent/agentic_loop.rs | 1 + src/agent/commands.rs | 89 +++++++++++++ src/agent/dispatcher.rs | 84 +++++++++++- src/agent/session.rs | 193 +++++++++++++++++++++++++++- src/agent/submission.rs | 11 ++ src/agent/thread_ops.rs | 69 ++++++++-- src/channels/channel.rs | 16 +++ src/channels/mod.rs | 2 +- src/channels/repl.rs | 14 ++ src/channels/wasm/wrapper.rs | 14 ++ src/channels/web/handlers/chat.rs | 2 + src/channels/web/mod.rs | 14 ++ src/channels/web/openai_compat.rs | 2 + src/channels/web/server.rs | 2 + src/channels/web/types.rs | 8 +- src/channels/web/util.rs | 99 ++++++++++++-- src/llm/anthropic_oauth.rs | 2 + src/llm/bedrock.rs | 7 + src/llm/codex_chatgpt.rs | 2 + src/llm/gemini_oauth.rs | 1 + src/llm/github_copilot.rs | 2 + src/llm/nearai_chat.rs | 7 + src/llm/openai_codex_provider.rs | 5 + src/llm/provider.rs | 8 ++ src/llm/reasoning.rs | 97 ++++++++++++-- src/llm/rig_adapter.rs | 7 + src/orchestrator/api.rs | 15 +++ src/worker/job.rs | 68 +++++++++- tests/openai_compat_integration.rs | 1 + tests/support/trace_llm.rs | 1 + 33 files changed, 871 insertions(+), 45 deletions(-) diff --git a/crates/ironclaw_common/src/event.rs b/crates/ironclaw_common/src/event.rs index 83592c95..256aba3d 100644 --- a/crates/ironclaw_common/src/event.rs +++ b/crates/ironclaw_common/src/event.rs @@ -7,6 +7,32 @@ use serde::{Deserialize, Serialize}; +/// A single tool decision in a reasoning update (SSE DTO). +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ToolDecisionDto { + pub tool_name: String, + pub rationale: String, +} + +impl ToolDecisionDto { + /// Parse a list of tool decisions from a JSON array value. + pub fn from_json_array(value: &serde_json::Value) -> Vec { + value + .as_array() + .map(|arr| { + arr.iter() + .filter_map(|d| { + Some(Self { + tool_name: d.get("tool_name")?.as_str()?.to_string(), + rationale: d.get("rationale")?.as_str()?.to_string(), + }) + }) + .collect() + }) + .unwrap_or_default() + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(tag = "type")] pub enum AppEvent { @@ -163,6 +189,23 @@ pub enum AppEvent { #[serde(skip_serializing_if = "Option::is_none")] message: Option, }, + + /// Agent reasoning update (why it chose specific tools). + #[serde(rename = "reasoning_update")] + ReasoningUpdate { + narrative: String, + decisions: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + thread_id: Option, + }, + + /// Reasoning update for a sandbox job. + #[serde(rename = "job_reasoning")] + JobReasoning { + job_id: String, + narrative: String, + decisions: Vec, + }, } impl AppEvent { @@ -191,6 +234,8 @@ impl AppEvent { Self::Suggestions { .. } => "suggestions", Self::TurnCost { .. } => "turn_cost", Self::ExtensionStatus { .. } => "extension_status", + Self::ReasoningUpdate { .. } => "reasoning_update", + Self::JobReasoning { .. } => "job_reasoning", } } } @@ -311,6 +356,16 @@ mod tests { status: String::new(), message: None, }, + AppEvent::ReasoningUpdate { + narrative: String::new(), + decisions: vec![], + thread_id: None, + }, + AppEvent::JobReasoning { + job_id: String::new(), + narrative: String::new(), + decisions: vec![], + }, ]; for variant in &variants { diff --git a/crates/ironclaw_common/src/lib.rs b/crates/ironclaw_common/src/lib.rs index 6822bad1..f52dc0aa 100644 --- a/crates/ironclaw_common/src/lib.rs +++ b/crates/ironclaw_common/src/lib.rs @@ -3,5 +3,5 @@ mod event; mod util; -pub use event::AppEvent; +pub use event::{AppEvent, ToolDecisionDto}; pub use util::truncate_preview; diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs index 7e950146..f51a8db1 100644 --- a/src/agent/agent_loop.rs +++ b/src/agent/agent_loop.rs @@ -1250,6 +1250,22 @@ impl Agent { command, message.channel ); + // /reasoning is special-cased here (not in handle_system_command) + // because it needs the session + thread_id to read turn reasoning + // data, which handle_system_command's signature doesn't provide. + if command == "reasoning" { + let result = self + .handle_reasoning_command(&args, &session, thread_id) + .await; + return match result { + SubmissionResult::Response { content } => Ok(Some(content)), + SubmissionResult::Ok { message } => Ok(message), + SubmissionResult::Error { message } => { + Ok(Some(format!("Error: {}", message))) + } + _ => Ok(Some(String::new())), + }; + } // Authorization checks (including restart channel check) are enforced in handle_system_command self.handle_system_command(&command, &args, &message.channel) .await diff --git a/src/agent/agentic_loop.rs b/src/agent/agentic_loop.rs index cc6fd486..e61856dc 100644 --- a/src/agent/agentic_loop.rs +++ b/src/agent/agentic_loop.rs @@ -414,6 +414,7 @@ mod tests { id: "call_1".to_string(), name: "echo".to_string(), arguments: serde_json::json!({}), + reasoning: None, }; let delegate = MockDelegate::new(vec![ tool_calls_output(vec![tool_call]), diff --git a/src/agent/commands.rs b/src/agent/commands.rs index b6aff3c0..e02b33db 100644 --- a/src/agent/commands.rs +++ b/src/agent/commands.rs @@ -465,6 +465,94 @@ impl Agent { } } + /// Handle `/reasoning [N|all]` — show reasoning history for the active thread. + pub(super) async fn handle_reasoning_command( + &self, + args: &[String], + session: &Arc>, + thread_id: Uuid, + ) -> SubmissionResult { + // Clone the turn data we need, then drop the session lock. + let turns_snapshot: Vec<( + usize, + Option, + Vec, + )>; + { + let sess = session.lock().await; + let thread = match sess.threads.get(&thread_id) { + Some(t) => t, + None => return SubmissionResult::error("No active thread."), + }; + + if thread.turns.is_empty() { + return SubmissionResult::ok_with_message("No turns yet."); + } + + // Parse argument: default=last turn, "all"=all turns, N=specific turn (1-based). + let selected: Vec<&crate::agent::session::Turn> = match args.first().map(|s| s.as_str()) + { + Some("all") => thread.turns.iter().collect(), + Some(n) => match n.parse::() { + Ok(0) => return SubmissionResult::error("Turn numbers start at 1."), + Ok(num) if num > thread.turns.len() => { + return SubmissionResult::error(format!( + "Turn {} does not exist (max: {}).", + num, + thread.turns.len() + )); + } + Ok(num) => vec![&thread.turns[num - 1]], + Err(_) => return SubmissionResult::error("Usage: /reasoning [N|all]"), + }, + None => { + // Default: last turn that has tool calls + match thread.turns.iter().rev().find(|t| !t.tool_calls.is_empty()) { + Some(t) => vec![t], + None => { + return SubmissionResult::ok_with_message("No turns with tool calls."); + } + } + } + }; + + turns_snapshot = selected + .into_iter() + .map(|t| (t.turn_number, t.narrative.clone(), t.tool_calls.clone())) + .collect(); + } + // Session lock is now dropped — format output without holding it. + + let mut output = String::new(); + for (turn_number, narrative, tool_calls) in &turns_snapshot { + output.push_str(&format!("--- Turn {} ---\n", turn_number + 1)); + if let Some(narrative) = narrative { + output.push_str(&format!("Reasoning: {}\n", narrative)); + } + if tool_calls.is_empty() { + output.push_str(" (no tool calls)\n"); + } else { + for tc in tool_calls { + let status = if tc.error.is_some() { + "error" + } else if tc.result.is_some() { + "ok" + } else { + "pending" + }; + output.push_str(&format!(" {} [{}]", tc.name, status)); + if let Some(ref rationale) = tc.rationale { + output.push_str(&format!(" — {}", rationale)); + } + output.push('\n'); + } + } + output.push('\n'); + } + + SubmissionResult::response(output.trim_end()) + } + /// Handle system commands that bypass thread-state checks entirely. pub(super) async fn handle_system_command( &self, @@ -480,6 +568,7 @@ impl Agent { " /version Show version info\n", " /tools List available tools\n", " /debug Toggle debug mode\n", + " /reasoning [N|all] Show agent reasoning for turns\n", " /ping Connectivity check\n", "\n", "Jobs:\n", diff --git a/src/agent/dispatcher.rs b/src/agent/dispatcher.rs index a195458d..cba84c35 100644 --- a/src/agent/dispatcher.rs +++ b/src/agent/dispatcher.rs @@ -420,6 +420,19 @@ impl<'a> LoopDelegate for ChatDelegate<'a> { content: Option, reason_ctx: &mut ReasoningContext, ) -> Result, Error> { + // Extract and sanitize the narrative before consuming `content`. + let narrative = content + .as_deref() + .filter(|c| !c.trim().is_empty()) + .map(|c| { + let sanitized = self + .agent + .safety() + .sanitize_tool_output("agent_narrative", c); + sanitized.content + }) + .filter(|c| !c.trim().is_empty()); + // Add the assistant message with tool_calls to context. // OpenAI protocol requires this before tool-result messages. reason_ctx @@ -440,6 +453,41 @@ impl<'a> LoopDelegate for ChatDelegate<'a> { ) .await; + // Build per-tool decisions for the reasoning update. + // Sanitize each rationale through SafetyLayer (parity with JobDelegate). + let decisions: Vec = tool_calls + .iter() + .filter_map(|tc| { + tc.reasoning.as_ref().map(|r| { + let sanitized = self + .agent + .safety() + .sanitize_tool_output("tool_rationale", r) + .content; + crate::channels::ToolDecision { + tool_name: tc.name.clone(), + rationale: sanitized, + } + }) + }) + .collect(); + + // Emit reasoning update to channels. + if narrative.is_some() || !decisions.is_empty() { + let _ = self + .agent + .channels + .send_status( + &self.message.channel, + StatusUpdate::ReasoningUpdate { + narrative: narrative.clone().unwrap_or_default(), + decisions: decisions.clone(), + }, + &self.message.metadata, + ) + .await; + } + // Record tool calls in the thread with sensitive params redacted. { let mut redacted_args: Vec = Vec::with_capacity(tool_calls.len()); @@ -455,8 +503,23 @@ impl<'a> LoopDelegate for ChatDelegate<'a> { if let Some(thread) = sess.threads.get_mut(&self.thread_id) && let Some(turn) = thread.last_turn_mut() { + // Set turn-level narrative. + if turn.narrative.is_none() { + turn.narrative = narrative; + } for (tc, safe_args) in tool_calls.iter().zip(redacted_args) { - turn.record_tool_call(&tc.name, safe_args); + let sanitized_rationale = tc.reasoning.as_ref().map(|r| { + self.agent + .safety() + .sanitize_tool_output("tool_rationale", r) + .content + }); + turn.record_tool_call_with_reasoning( + &tc.name, + safe_args, + sanitized_rationale, + Some(tc.id.clone()), + ); } } } @@ -726,7 +789,7 @@ impl<'a> LoopDelegate for ChatDelegate<'a> { if let Some(thread) = sess.threads.get_mut(&self.thread_id) && let Some(turn) = thread.last_turn_mut() { - turn.record_tool_error(error_msg.clone()); + turn.record_tool_error_for(&tc.id, error_msg.clone()); } } reason_ctx @@ -852,16 +915,19 @@ impl<'a> LoopDelegate for ChatDelegate<'a> { Err(e) => format!("Tool '{}' failed: {}", tc.name, e), }; - // Record sanitized result in thread + // Record sanitized result in thread (identity-based matching). { let mut sess = self.session.lock().await; if let Some(thread) = sess.threads.get_mut(&self.thread_id) && let Some(turn) = thread.last_turn_mut() { if is_tool_error { - turn.record_tool_error(result_content.clone()); + turn.record_tool_error_for(&tc.id, result_content.clone()); } else { - turn.record_tool_result(serde_json::json!(result_content)); + turn.record_tool_result_for( + &tc.id, + serde_json::json!(result_content), + ); } } } @@ -1462,11 +1528,13 @@ mod tests { id: "call_2".to_string(), name: "http".to_string(), arguments: serde_json::json!({"url": "https://example.com"}), + reasoning: None, }, ToolCall { id: "call_3".to_string(), name: "echo".to_string(), arguments: serde_json::json!({"message": "done"}), + reasoning: None, }, ], user_timezone: None, @@ -1652,6 +1720,7 @@ mod tests { id: "call_1".to_string(), name: "echo".to_string(), arguments: serde_json::json!({"message": "hi"}), + reasoning: None, }], ), ChatMessage::tool_result("call_1", "echo", "hi"), @@ -1744,11 +1813,13 @@ mod tests { id: "c1".to_string(), name: "http".to_string(), arguments: serde_json::json!({}), + reasoning: None, }, ToolCall { id: "c2".to_string(), name: "echo".to_string(), arguments: serde_json::json!({}), + reasoning: None, }, ], ), @@ -1782,6 +1853,7 @@ mod tests { id: "c1".to_string(), name: "echo".to_string(), arguments: serde_json::json!({}), + reasoning: None, }], ), ChatMessage::tool_result("c1", "echo", "done"), @@ -1912,6 +1984,7 @@ mod tests { id: crate::llm::generate_tool_call_id(0, 0), name: "echo".to_string(), arguments: serde_json::json!({"message": "looping"}), + reasoning: None, }], input_tokens: 0, output_tokens: 5, @@ -2065,6 +2138,7 @@ mod tests { id: crate::llm::generate_tool_call_id(0, 0), name: "nonexistent_tool".to_string(), arguments: serde_json::json!({}), + reasoning: None, }], input_tokens: 0, output_tokens: 5, diff --git a/src/agent/session.rs b/src/agent/session.rs index 7ec2023f..6c873e46 100644 --- a/src/agent/session.rs +++ b/src/agent/session.rs @@ -449,6 +449,7 @@ impl Thread { id: call_id.clone(), name: tc.name.clone(), arguments: tc.parameters.clone(), + reasoning: None, }) .collect(); @@ -522,7 +523,12 @@ impl Thread { && let Some(ref tcs) = assistant_msg.tool_calls { for tc in tcs { - turn.record_tool_call(&tc.name, tc.arguments.clone()); + turn.record_tool_call_with_reasoning( + &tc.name, + tc.arguments.clone(), + tc.reasoning.clone(), + Some(tc.id.clone()), + ); } } @@ -602,6 +608,10 @@ pub struct Turn { pub completed_at: Option>, /// Error message (if failed). pub error: Option, + /// Agent's reasoning narrative for this turn. + /// Cleaned via `clean_response` and sanitized through `SafetyLayer` before storage. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub narrative: Option, /// Transient image content parts for multimodal LLM input. /// Not serialized — images are only needed for the current LLM call. /// The text description in `user_input` persists for compaction/context. @@ -621,6 +631,7 @@ impl Turn { started_at: Utc::now(), completed_at: None, error: None, + narrative: None, image_content_parts: Vec::new(), } } @@ -656,6 +667,26 @@ impl Turn { parameters: params, result: None, error: None, + rationale: None, + tool_call_id: None, + }); + } + + /// Record a tool call with reasoning context. + pub fn record_tool_call_with_reasoning( + &mut self, + name: impl Into, + params: serde_json::Value, + rationale: Option, + tool_call_id: Option, + ) { + self.tool_calls.push(TurnToolCall { + name: name.into(), + parameters: params, + result: None, + error: None, + rationale, + tool_call_id, }); } @@ -672,6 +703,60 @@ impl Turn { call.error = Some(error.into()); } } + + /// Record a tool result by tool_call_id, with fallback to first pending call. + pub fn record_tool_result_for(&mut self, tool_call_id: &str, result: serde_json::Value) { + if let Some(call) = self + .tool_calls + .iter_mut() + .find(|c| c.tool_call_id.as_deref() == Some(tool_call_id)) + { + call.result = Some(result); + } else if let Some(call) = self + .tool_calls + .iter_mut() + .find(|c| c.result.is_none() && c.error.is_none()) + { + tracing::debug!( + tool_call_id = %tool_call_id, + fallback_tool = %call.name, + "tool_call_id not found, falling back to first pending call" + ); + call.result = Some(result); + } else { + tracing::warn!( + tool_call_id = %tool_call_id, + "Tool result dropped: no matching or pending tool call" + ); + } + } + + /// Record a tool error by tool_call_id, with fallback to first pending call. + pub fn record_tool_error_for(&mut self, tool_call_id: &str, error: impl Into) { + if let Some(call) = self + .tool_calls + .iter_mut() + .find(|c| c.tool_call_id.as_deref() == Some(tool_call_id)) + { + call.error = Some(error.into()); + } else if let Some(call) = self + .tool_calls + .iter_mut() + .find(|c| c.result.is_none() && c.error.is_none()) + { + tracing::debug!( + tool_call_id = %tool_call_id, + fallback_tool = %call.name, + "tool_call_id not found, falling back to first pending call" + ); + call.error = Some(error.into()); + } else { + tracing::warn!( + tool_call_id = %tool_call_id, + "Tool error dropped: no matching or pending tool call" + ); + } + } } /// Record of a tool call made during a turn. @@ -685,6 +770,12 @@ pub struct TurnToolCall { pub result: Option, /// Error from the tool (if failed). pub error: Option, + /// Agent's reasoning for choosing this tool. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rationale: Option, + /// The tool_call_id from the LLM, for identity-based result matching. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub tool_call_id: Option, } #[cfg(test)] @@ -1309,6 +1400,7 @@ mod tests { id: "call_0".to_string(), name: "search".to_string(), arguments: serde_json::json!({"q": "test"}), + reasoning: None, }; let messages = vec![ ChatMessage::user("Find test"), @@ -1339,6 +1431,7 @@ mod tests { id: "call_0".to_string(), name: "http".to_string(), arguments: serde_json::json!({}), + reasoning: None, }; let messages = vec![ ChatMessage::user("Fetch URL"), @@ -1404,11 +1497,13 @@ mod tests { id: "call_a".to_string(), name: "search".to_string(), arguments: serde_json::json!({"q": "data"}), + reasoning: None, }; let tc2 = ToolCall { id: "call_b".to_string(), name: "write".to_string(), arguments: serde_json::json!({"path": "out.txt"}), + reasoning: None, }; let messages = vec![ ChatMessage::user("Find and save"), @@ -1620,4 +1715,100 @@ mod tests { let merged = thread.drain_pending_messages().unwrap(); assert_eq!(merged, "failed batch\nnew msg"); } + + #[test] + fn test_record_tool_result_for_by_id() { + let mut turn = Turn::new(0, "test"); + turn.record_tool_call_with_reasoning( + "tool_a", + serde_json::json!({}), + None, + Some("id_a".into()), + ); + turn.record_tool_call_with_reasoning( + "tool_b", + serde_json::json!({}), + None, + Some("id_b".into()), + ); + + // Record result for second tool by ID + turn.record_tool_result_for("id_b", serde_json::json!("result_b")); + assert!(turn.tool_calls[0].result.is_none()); + assert_eq!( + turn.tool_calls[1].result.as_ref().unwrap(), + &serde_json::json!("result_b") + ); + } + + #[test] + fn test_record_tool_error_for_by_id() { + let mut turn = Turn::new(0, "test"); + turn.record_tool_call_with_reasoning( + "tool_a", + serde_json::json!({}), + None, + Some("id_a".into()), + ); + turn.record_tool_call_with_reasoning( + "tool_b", + serde_json::json!({}), + None, + Some("id_b".into()), + ); + + turn.record_tool_error_for("id_a", "failed"); + assert_eq!(turn.tool_calls[0].error.as_deref(), Some("failed")); + assert!(turn.tool_calls[1].error.is_none()); + } + + #[test] + fn test_record_tool_result_for_fallback_to_pending() { + let mut turn = Turn::new(0, "test"); + turn.record_tool_call_with_reasoning( + "tool_a", + serde_json::json!({}), + None, + Some("id_a".into()), + ); + turn.record_tool_call_with_reasoning( + "tool_b", + serde_json::json!({}), + None, + Some("id_b".into()), + ); + + // First tool already has a result + turn.tool_calls[0].result = Some(serde_json::json!("done")); + + // Unknown ID should fall back to first pending (tool_b) + turn.record_tool_result_for("unknown_id", serde_json::json!("fallback")); + assert_eq!( + turn.tool_calls[0].result.as_ref().unwrap(), + &serde_json::json!("done") + ); + assert_eq!( + turn.tool_calls[1].result.as_ref().unwrap(), + &serde_json::json!("fallback") + ); + } + + #[test] + fn test_record_tool_result_for_no_pending_is_noop() { + let mut turn = Turn::new(0, "test"); + turn.record_tool_call_with_reasoning( + "tool_a", + serde_json::json!({}), + None, + Some("id_a".into()), + ); + turn.tool_calls[0].result = Some(serde_json::json!("done")); + + // No pending calls, unknown ID — should be a no-op + turn.record_tool_result_for("unknown_id", serde_json::json!("lost")); + assert_eq!( + turn.tool_calls[0].result.as_ref().unwrap(), + &serde_json::json!("done") + ); + } } diff --git a/src/agent/submission.rs b/src/agent/submission.rs index 8594c969..5a81e0bf 100644 --- a/src/agent/submission.rs +++ b/src/agent/submission.rs @@ -92,6 +92,17 @@ impl SubmissionParser { args: vec![], }; } + if lower == "/reasoning" || lower.starts_with("/reasoning ") { + let args: Vec = trimmed + .split_whitespace() + .skip(1) + .map(|s| s.to_string()) + .collect(); + return Submission::SystemCommand { + command: "reasoning".to_string(), + args, + }; + } if lower == "/restart" { tracing::debug!("[SubmissionParser::parse] Recognized /restart command"); return Submission::SystemCommand { diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index b2820e7e..11f211f9 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -513,10 +513,10 @@ impl Agent { }; thread.complete_turn(&response); - let (turn_number, tool_calls) = thread + let (turn_number, tool_calls, narrative) = thread .turns .last() - .map(|t| (t.turn_number, t.tool_calls.clone())) + .map(|t| (t.turn_number, t.tool_calls.clone(), t.narrative.clone())) .unwrap_or_default(); let _ = self .channels @@ -534,6 +534,7 @@ impl Agent { &message.user_id, turn_number, &tool_calls, + narrative.as_deref(), ) .await; self.persist_assistant_response( @@ -725,7 +726,9 @@ impl Agent { /// /// Stored between the user and assistant messages so that /// `build_turns_from_db_messages` can reconstruct the tool call history. - /// Content is a JSON array of tool call summaries. + /// Content is a JSON object: `{ "calls": [...], "narrative": "..." }`. + /// The `calls` array contains tool call summaries with optional `rationale` + /// and `tool_call_id` fields. Legacy rows may be plain JSON arrays. pub(super) async fn persist_tool_calls( &self, thread_id: Uuid, @@ -733,6 +736,7 @@ impl Agent { user_id: &str, turn_number: usize, tool_calls: &[crate::agent::session::TurnToolCall], + narrative: Option<&str>, ) { if tool_calls.is_empty() { return; @@ -767,11 +771,30 @@ impl Agent { if let Some(ref error) = tc.error { obj["error"] = serde_json::Value::String(truncate_preview(error, 200)); } + if let Some(ref rationale) = tc.rationale { + obj["rationale"] = serde_json::Value::String(truncate_preview(rationale, 500)); + } + if let Some(ref tool_call_id) = tc.tool_call_id { + obj["tool_call_id"] = + serde_json::Value::String(truncate_preview(tool_call_id, 128)); + } obj }) .collect(); - let content = match serde_json::to_string(&summaries) { + // Wrap in an object with optional narrative so it can be reconstructed. + // safety: no byte-index slicing here; comment describes JSON shape + let wrapper = if let Some(n) = narrative { + serde_json::json!({ + "narrative": truncate_preview(n, 1000), + "calls": summaries, + }) + } else { + serde_json::json!({ + "calls": summaries, + }) + }; + let content = match serde_json::to_string(&wrapper) { Ok(c) => c, Err(e) => { tracing::warn!("Failed to serialize tool calls: {}", e); @@ -1104,9 +1127,12 @@ impl Agent { && let Some(turn) = thread.last_turn_mut() { if is_tool_error { - turn.record_tool_error(result_content.clone()); + turn.record_tool_error_for(&pending.tool_call_id, result_content.clone()); } else { - turn.record_tool_result(serde_json::json!(result_content)); + turn.record_tool_result_for( + &pending.tool_call_id, + serde_json::json!(result_content), + ); } } } @@ -1358,9 +1384,12 @@ impl Agent { && let Some(turn) = thread.last_turn_mut() { if is_deferred_error { - turn.record_tool_error(deferred_content.clone()); + turn.record_tool_error_for(&tc.id, deferred_content.clone()); } else { - turn.record_tool_result(serde_json::json!(deferred_content)); + turn.record_tool_result_for( + &tc.id, + serde_json::json!(deferred_content), + ); } } } @@ -1459,10 +1488,10 @@ impl Agent { let (response, suggestions) = crate::agent::dispatcher::extract_suggestions(&response); thread.complete_turn(&response); - let (turn_number, tool_calls) = thread + let (turn_number, tool_calls, narrative) = thread .turns .last() - .map(|t| (t.turn_number, t.tool_calls.clone())) + .map(|t| (t.turn_number, t.tool_calls.clone(), t.narrative.clone())) .unwrap_or_default(); // User message already persisted at turn start; save tool calls then assistant response self.persist_tool_calls( @@ -1471,6 +1500,7 @@ impl Agent { &message.user_id, turn_number, &tool_calls, + narrative.as_deref(), ) .await; self.persist_assistant_response( @@ -1816,7 +1846,20 @@ fn rebuild_chat_messages_from_db( "assistant" => result.push(ChatMessage::assistant(&msg.content)), "tool_calls" => { // Try to parse the enriched JSON and rebuild tool messages. - if let Ok(calls) = serde_json::from_str::>(&msg.content) { + // Supports two formats: + // - Old: plain JSON array of tool call summaries + // - New: wrapped object { "calls": [...], "narrative": "..." } + let calls: Vec = + match serde_json::from_str::(&msg.content) { + Ok(serde_json::Value::Array(arr)) => arr, + Ok(serde_json::Value::Object(obj)) => obj + .get("calls") + .and_then(|v| v.as_array()) + .cloned() + .unwrap_or_default(), + _ => Vec::new(), + }; + { if calls.is_empty() { continue; } @@ -1839,6 +1882,10 @@ fn rebuild_chat_messages_from_db( .get("parameters") .cloned() .unwrap_or(serde_json::json!({})), + reasoning: c + .get("rationale") + .and_then(|v| v.as_str()) + .map(String::from), }) .collect(); diff --git a/src/channels/channel.rs b/src/channels/channel.rs index 9bcee12e..784b6bcf 100644 --- a/src/channels/channel.rs +++ b/src/channels/channel.rs @@ -265,6 +265,15 @@ impl OutgoingResponse { } } +/// A single tool decision within a reasoning update. +#[derive(Debug, Clone)] +pub struct ToolDecision { + /// Tool name. + pub tool_name: String, + /// Agent's reasoning for choosing this tool. + pub rationale: String, +} + /// Status update types for showing agent activity. #[derive(Debug, Clone)] pub enum StatusUpdate { @@ -333,6 +342,13 @@ pub enum StatusUpdate { }, /// Suggested follow-up messages for the user. Suggestions { suggestions: Vec }, + /// Agent reasoning update (why it chose specific tools). + ReasoningUpdate { + /// Human-readable summary of the agent's decision. + narrative: String, + /// Per-tool decisions. + decisions: Vec, + }, /// Per-turn token usage and cost summary (shown as subtle metadata). TurnCost { input_tokens: u64, diff --git a/src/channels/mod.rs b/src/channels/mod.rs index c0230692..46e25514 100644 --- a/src/channels/mod.rs +++ b/src/channels/mod.rs @@ -39,7 +39,7 @@ mod webhook_server; pub use channel::{ AttachmentKind, Channel, ChannelSecretUpdater, IncomingAttachment, IncomingMessage, - MessageStream, OutgoingResponse, StatusUpdate, routing_target_from_metadata, + MessageStream, OutgoingResponse, StatusUpdate, ToolDecision, routing_target_from_metadata, }; pub use http::{HttpChannel, HttpChannelState}; pub use manager::ChannelManager; diff --git a/src/channels/repl.rs b/src/channels/repl.rs index 055dc3ad..61c68d13 100644 --- a/src/channels/repl.rs +++ b/src/channels/repl.rs @@ -75,6 +75,7 @@ const SLASH_COMMANDS: &[&str] = &[ "/suggest", "/thread", "/resume", + "/reasoning", ]; /// Rustyline helper for slash-command tab completion. @@ -841,6 +842,19 @@ impl Channel for ReplChannel { StatusUpdate::Suggestions { .. } => { // Suggestions are only rendered by the web gateway } + StatusUpdate::ReasoningUpdate { + narrative, + decisions, + } => { + if !narrative.is_empty() { + let display = truncate_for_preview(&narrative, CLI_STATUS_MAX); + eprintln!(" \x1b[94m\u{25B6} {display}\x1b[0m"); + } + for d in &decisions { + let display = truncate_for_preview(&d.rationale, CLI_STATUS_MAX); + eprintln!(" \x1b[90m\u{2192} {}: {display}\x1b[0m", d.tool_name); + } + } StatusUpdate::TurnCost { .. } => { // Cost display is handled by the TUI channel } diff --git a/src/channels/wasm/wrapper.rs b/src/channels/wasm/wrapper.rs index 65e4de88..a0f9689f 100644 --- a/src/channels/wasm/wrapper.rs +++ b/src/channels/wasm/wrapper.rs @@ -3061,6 +3061,20 @@ fn status_to_wit( }, // Suggestions and turn cost are web-gateway-only; skip for WASM channels StatusUpdate::Suggestions { .. } | StatusUpdate::TurnCost { .. } => return None, + StatusUpdate::ReasoningUpdate { + narrative, + decisions, + } => { + let mut msg = narrative.clone(); + for d in decisions { + msg.push_str(&format!("\n → {}: {}", d.tool_name, d.rationale)); + } + wit_channel::StatusUpdate { + status: wit_channel::StatusType::Status, + message: msg, + metadata_json, + } + } }) } diff --git a/src/channels/web/handlers/chat.rs b/src/channels/web/handlers/chat.rs index de4b3155..bc4e3dbc 100644 --- a/src/channels/web/handlers/chat.rs +++ b/src/channels/web/handlers/chat.rs @@ -398,8 +398,10 @@ pub async fn chat_history_handler( truncate_preview(&s, 500) }), error: tc.error.clone(), + rationale: tc.rationale.clone(), }) .collect(), + narrative: t.narrative.clone(), }) .collect(); diff --git a/src/channels/web/mod.rs b/src/channels/web/mod.rs index 6a97e8b8..63aedaa0 100644 --- a/src/channels/web/mod.rs +++ b/src/channels/web/mod.rs @@ -489,6 +489,20 @@ impl Channel for GatewayChannel { }, StatusUpdate::Suggestions { suggestions } => AppEvent::Suggestions { suggestions, + thread_id: thread_id.clone(), + }, + StatusUpdate::ReasoningUpdate { + narrative, + decisions, + } => AppEvent::ReasoningUpdate { + narrative, + decisions: decisions + .into_iter() + .map(|d| crate::channels::web::types::ToolDecisionDto { + tool_name: d.tool_name, + rationale: d.rationale, + }) + .collect(), thread_id, }, StatusUpdate::TurnCost { diff --git a/src/channels/web/openai_compat.rs b/src/channels/web/openai_compat.rs index 55b7c854..0c0f1a9e 100644 --- a/src/channels/web/openai_compat.rs +++ b/src/channels/web/openai_compat.rs @@ -231,6 +231,7 @@ pub fn convert_messages(messages: &[OpenAiMessage]) -> Result, name: tc.function.name.clone(), arguments: serde_json::from_str(&tc.function.arguments) .unwrap_or(serde_json::Value::Object(Default::default())), + reasoning: None, }) .collect(); Ok(ChatMessage::assistant_with_tool_calls( @@ -954,6 +955,7 @@ mod tests { id: "call_abc".to_string(), name: "search".to_string(), arguments: serde_json::json!({"query": "rust"}), + reasoning: None, }]; let converted = convert_tool_calls_to_openai(&calls); diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 5b092312..c24ceb16 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -1725,8 +1725,10 @@ async fn chat_history_handler( truncate_preview(&s, 500) }), error: tc.error.clone(), + rationale: tc.rationale.clone(), }) .collect(), + narrative: t.narrative.clone(), }) .collect(); diff --git a/src/channels/web/types.rs b/src/channels/web/types.rs index fe18a824..8698c030 100644 --- a/src/channels/web/types.rs +++ b/src/channels/web/types.rs @@ -63,6 +63,9 @@ pub struct TurnInfo { pub started_at: String, pub completed_at: Option, pub tool_calls: Vec, + /// Agent's reasoning narrative for this turn. + #[serde(skip_serializing_if = "Option::is_none")] + pub narrative: Option, } #[derive(Debug, Serialize)] @@ -74,6 +77,9 @@ pub struct ToolCallInfo { pub result_preview: Option, #[serde(skip_serializing_if = "Option::is_none")] pub error: Option, + /// Agent's reasoning for choosing this tool. + #[serde(skip_serializing_if = "Option::is_none")] + pub rationale: Option, } #[derive(Debug, Serialize)] @@ -116,7 +122,7 @@ pub struct ApprovalRequest { // --- App Event (re-exported from ironclaw_common) --- -pub use ironclaw_common::AppEvent; +pub use ironclaw_common::{AppEvent, ToolDecisionDto}; // --- Memory --- diff --git a/src/channels/web/util.rs b/src/channels/web/util.rs index ed70c5ce..2e4ffe3b 100644 --- a/src/channels/web/util.rs +++ b/src/channels/web/util.rs @@ -4,6 +4,21 @@ use crate::channels::web::types::{ToolCallInfo, TurnInfo}; pub use ironclaw_common::truncate_preview; +/// Parse tool call summary JSON objects into `ToolCallInfo` structs. +fn parse_tool_call_infos(calls: &[serde_json::Value]) -> Vec { + calls + .iter() + .map(|c| ToolCallInfo { + name: c["name"].as_str().unwrap_or("unknown").to_string(), + has_result: c.get("result_preview").is_some_and(|v| !v.is_null()), + has_error: c.get("error").is_some_and(|v| !v.is_null()), + result_preview: c["result_preview"].as_str().map(String::from), + error: c["error"].as_str().map(String::from), + rationale: c["rationale"].as_str().map(String::from), + }) + .collect() +} + /// Build TurnInfo pairs from flat DB messages (user/tool_calls/assistant triples). /// /// Handles three message patterns: @@ -27,6 +42,7 @@ pub fn build_turns_from_db_messages( started_at: msg.created_at.to_rfc3339(), completed_at: None, tool_calls: Vec::new(), + narrative: None, }; // Check if next message is a tool_calls record @@ -34,18 +50,28 @@ pub fn build_turns_from_db_messages( && next.role == "tool_calls" { let tc_msg = iter.next().expect("peeked"); - match serde_json::from_str::>(&tc_msg.content) { - Ok(calls) => { - turn.tool_calls = calls - .iter() - .map(|c| ToolCallInfo { - name: c["name"].as_str().unwrap_or("unknown").to_string(), - has_result: c.get("result_preview").is_some(), - has_error: c.get("error").is_some(), - result_preview: c["result_preview"].as_str().map(String::from), - error: c["error"].as_str().map(String::from), - }) - .collect(); + // Parse tool_calls JSON — supports two formats: + // safety: no byte-index slicing; comment describes JSON shape + match serde_json::from_str::(&tc_msg.content) { + Ok(serde_json::Value::Array(calls)) => { + // Old format: plain array + turn.tool_calls = parse_tool_call_infos(&calls); + } + Ok(serde_json::Value::Object(obj)) => { + // New wrapped format with narrative + turn.narrative = obj + .get("narrative") + .and_then(|v| v.as_str()) + .map(String::from); + if let Some(serde_json::Value::Array(calls)) = obj.get("calls") { + turn.tool_calls = parse_tool_call_infos(calls); + } + } + Ok(_) => { + tracing::warn!( + message_id = %tc_msg.id, + "Unexpected tool_calls JSON shape in DB, skipping" + ); } Err(e) => { tracing::warn!( @@ -83,6 +109,7 @@ pub fn build_turns_from_db_messages( started_at: msg.created_at.to_rfc3339(), completed_at: Some(msg.created_at.to_rfc3339()), tool_calls: Vec::new(), + narrative: None, }); turn_number += 1; } @@ -201,4 +228,52 @@ mod tests { assert!(turns[0].tool_calls.is_empty()); assert_eq!(turns[0].state, "Completed"); } + + #[test] + fn test_build_turns_with_wrapped_tool_calls_format() { + let tc_json = serde_json::json!({ + "narrative": "Searching memory for context before proceeding.", + "calls": [ + {"name": "memory_search", "result_preview": "found 3 items", "rationale": "consult prior context"}, + {"name": "shell", "error": "permission denied"} + ] + }); + let messages = vec![ + make_msg("user", "Find info", 0), + make_msg("tool_calls", &tc_json.to_string(), 500), + make_msg("assistant", "Here's what I found", 1000), + ]; + let turns = build_turns_from_db_messages(&messages); + assert_eq!(turns.len(), 1); + assert_eq!( + turns[0].narrative.as_deref(), + Some("Searching memory for context before proceeding.") + ); + assert_eq!(turns[0].tool_calls.len(), 2); + assert_eq!(turns[0].tool_calls[0].name, "memory_search"); + assert_eq!( + turns[0].tool_calls[0].rationale.as_deref(), + Some("consult prior context") + ); + assert!(turns[0].tool_calls[0].has_result); + assert_eq!(turns[0].tool_calls[1].name, "shell"); + assert!(turns[0].tool_calls[1].has_error); + assert_eq!(turns[0].response.as_deref(), Some("Here's what I found")); + } + + #[test] + fn test_build_turns_wrapped_format_without_narrative() { + let tc_json = serde_json::json!({ + "calls": [{"name": "echo", "result_preview": "hello"}] + }); + let messages = vec![ + make_msg("user", "Say hi", 0), + make_msg("tool_calls", &tc_json.to_string(), 500), + make_msg("assistant", "Done", 1000), + ]; + let turns = build_turns_from_db_messages(&messages); + assert_eq!(turns.len(), 1); + assert!(turns[0].narrative.is_none()); + assert_eq!(turns[0].tool_calls.len(), 1); + } } diff --git a/src/llm/anthropic_oauth.rs b/src/llm/anthropic_oauth.rs index 490fbc3f..c94c90e5 100644 --- a/src/llm/anthropic_oauth.rs +++ b/src/llm/anthropic_oauth.rs @@ -575,6 +575,7 @@ fn extract_response_content(response: &AnthropicResponse) -> (Option, Ve id: id.clone(), name: name.clone(), arguments: input.clone(), + reasoning: None, }); } } @@ -623,6 +624,7 @@ mod tests { id: "call_1".to_string(), name: "search".to_string(), arguments: serde_json::json!({"q": "test"}), + reasoning: None, }]; let messages = vec![ ChatMessage::user("Search for test"), diff --git a/src/llm/bedrock.rs b/src/llm/bedrock.rs index 5d6e121e..b5f7badd 100644 --- a/src/llm/bedrock.rs +++ b/src/llm/bedrock.rs @@ -522,6 +522,7 @@ fn extract_content_blocks( id: tu.tool_use_id().to_string(), name: tu.name().to_string(), arguments: document_to_json(tu.input()), + reasoning: None, }); } // Ignore reasoning, citations, images, etc. @@ -759,11 +760,13 @@ mod tests { id: "call_1".to_string(), name: "echo".to_string(), arguments: serde_json::json!({"text": "hi"}), + reasoning: None, }; let tc2 = crate::llm::provider::ToolCall { id: "call_2".to_string(), name: "time".to_string(), arguments: serde_json::json!({}), + reasoning: None, }; let messages = vec![ @@ -802,6 +805,7 @@ mod tests { id: "call_1".to_string(), name: "search".to_string(), arguments: serde_json::json!({"query": "test"}), + reasoning: None, }; let messages = vec![ @@ -825,6 +829,7 @@ mod tests { id: "call_1".to_string(), name: "echo".to_string(), arguments: serde_json::json!({}), + reasoning: None, }; let messages = vec![ @@ -989,11 +994,13 @@ mod tests { id: "call_abc".to_string(), name: "get_weather".to_string(), arguments: serde_json::json!({"city": "NYC"}), + reasoning: None, }; let tc2 = crate::llm::provider::ToolCall { id: "call_def".to_string(), name: "get_time".to_string(), arguments: serde_json::json!({"tz": "EST"}), + reasoning: None, }; let messages = vec![ diff --git a/src/llm/codex_chatgpt.rs b/src/llm/codex_chatgpt.rs index 56cb3378..e7dcf40d 100644 --- a/src/llm/codex_chatgpt.rs +++ b/src/llm/codex_chatgpt.rs @@ -732,6 +732,7 @@ impl LlmProvider for CodexChatGptProvider { id: tc.call_id, name: tc.name, arguments: args, + reasoning: None, } }) .collect(); @@ -825,6 +826,7 @@ mod tests { id: "call_1".to_string(), name: "search".to_string(), arguments: json!({"query": "rust"}), + reasoning: None, }; let msg = ChatMessage::assistant_with_tool_calls(Some("thinking...".into()), vec![tc]); let items = CodexChatGptProvider::message_to_input_items(&msg); diff --git a/src/llm/gemini_oauth.rs b/src/llm/gemini_oauth.rs index b36eb595..a19eec12 100644 --- a/src/llm/gemini_oauth.rs +++ b/src/llm/gemini_oauth.rs @@ -1898,6 +1898,7 @@ impl GeminiOauthProvider { id, name, arguments: args, + reasoning: None, }); } } diff --git a/src/llm/github_copilot.rs b/src/llm/github_copilot.rs index b173191a..c7a24b1a 100644 --- a/src/llm/github_copilot.rs +++ b/src/llm/github_copilot.rs @@ -596,6 +596,7 @@ fn extract_choice_content(choice: &OpenAiChoice) -> (Option, Vec Result { id: state.call_id, name: state.name, arguments, + reasoning: None, }); } else { // Fallback: extract directly from the item @@ -650,6 +651,7 @@ fn parse_sse_response(body: &str) -> Result { id: call_id, name, arguments, + reasoning: None, }); } } @@ -727,6 +729,7 @@ fn parse_sse_response(body: &str) -> Result { id: state.call_id, name: state.name, arguments, + reasoning: None, }); } } @@ -822,11 +825,13 @@ mod tests { id: "call_1".to_string(), name: "search".to_string(), arguments: serde_json::json!({"query": "test"}), + reasoning: None, }, ToolCall { id: "call_2".to_string(), name: "read".to_string(), arguments: serde_json::json!({"path": "/tmp"}), + reasoning: None, }, ]; let msg = diff --git a/src/llm/provider.rs b/src/llm/provider.rs index bb45ec68..8afd914a 100644 --- a/src/llm/provider.rs +++ b/src/llm/provider.rs @@ -231,6 +231,10 @@ pub struct ToolCall { pub id: String, pub name: String, pub arguments: serde_json::Value, + /// Optional reasoning for why this tool was chosen — supplied by the provider + /// or derived from the shared response content as a fallback. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reasoning: Option, } /// Generate a tool-call ID that satisfies all providers. @@ -637,6 +641,7 @@ mod tests { id: "call_1".to_string(), name: "echo".to_string(), arguments: serde_json::json!({}), + reasoning: None, }; let mut messages = vec![ ChatMessage::user("hello"), @@ -680,6 +685,7 @@ mod tests { id: "call_1".to_string(), name: "echo".to_string(), arguments: serde_json::json!({}), + reasoning: None, }; let mut messages = vec![ ChatMessage::user("test"), @@ -705,11 +711,13 @@ mod tests { id: "call_sel_1".to_string(), name: "search".to_string(), arguments: serde_json::json!({"q": "test"}), + reasoning: None, }; let tc2 = ToolCall { id: "call_sel_2".to_string(), name: "http".to_string(), arguments: serde_json::json!({"url": "https://example.com"}), + reasoning: None, }; let mut messages = vec![ ChatMessage::system("You are a helpful assistant."), diff --git a/src/llm/reasoning.rs b/src/llm/reasoning.rs index cbec297b..77905f95 100644 --- a/src/llm/reasoning.rs +++ b/src/llm/reasoning.rs @@ -525,17 +525,35 @@ impl Reasoning { let response = self.llm.complete_with_tools(request).await?; - let reasoning = response.content.unwrap_or_default(); + let shared_reasoning = response + .content + .map(|c| { + let pre_truncated = truncate_at_tool_tags(&c); + clean_response(&pre_truncated) + }) + .unwrap_or_default(); let selections: Vec = response .tool_calls .into_iter() - .map(|tool_call| ToolSelection { - tool_name: tool_call.name, - parameters: tool_call.arguments, - reasoning: reasoning.clone(), - alternatives: vec![], - tool_call_id: tool_call.id, + .map(|tool_call| { + // Prefer per-tool reasoning if the provider supplied it, + // otherwise fall back to the shared response content. + let rationale = tool_call + .reasoning + .map(|r| { + let pre_truncated = truncate_at_tool_tags(&r); + clean_response(&pre_truncated) + }) + .filter(|r| !r.trim().is_empty()) + .unwrap_or_else(|| shared_reasoning.clone()); + ToolSelection { + tool_name: tool_call.name, + parameters: tool_call.arguments, + reasoning: rationale, + alternatives: vec![], + tool_call_id: tool_call.id, + } }) .collect(); @@ -664,13 +682,36 @@ Respond in JSON format: // If there were tool calls, return them for execution if !response.tool_calls.is_empty() { + let narrative = response.content.map(|c| { + let pre_truncated = truncate_at_tool_tags(&c); + clean_response(&pre_truncated) + }); + // Populate per-tool reasoning from the shared narrative when the + // provider did not supply per-tool rationale. + let tool_calls: Vec = response + .tool_calls + .into_iter() + .map(|mut tc| { + if tc.reasoning.as_ref().is_none_or(|r| r.trim().is_empty()) { + tc.reasoning = narrative.as_ref().filter(|n| !n.is_empty()).cloned(); + } else { + // Clean provider-supplied per-tool reasoning the same way + // we clean the shared narrative (strip thinking/tool tags). + tc.reasoning = tc + .reasoning + .map(|r| { + let pre_truncated = truncate_at_tool_tags(&r); + clean_response(&pre_truncated) + }) + .filter(|r| !r.trim().is_empty()); + } + tc + }) + .collect(); return Ok(RespondOutput { result: RespondResult::ToolCalls { - tool_calls: response.tool_calls, - content: response.content.map(|c| { - let pre_truncated = truncate_at_tool_tags(&c); - clean_response(&pre_truncated) - }), + tool_calls, + content: narrative, }, usage, }); @@ -1350,6 +1391,7 @@ fn recover_tool_calls_from_content( ), name: name.to_string(), arguments, + reasoning: None, }); continue; } @@ -1364,6 +1406,7 @@ fn recover_tool_calls_from_content( ), name: name.to_string(), arguments: serde_json::Value::Object(Default::default()), + reasoning: None, }); } } @@ -1401,6 +1444,7 @@ fn recover_tool_calls_from_content( ), name: name.to_string(), arguments, + reasoning: None, }); remaining = &args_start[bracket_end + 1..]; continue; @@ -1412,6 +1456,7 @@ fn recover_tool_calls_from_content( id: super::provider::generate_tool_call_id(calls.len(), RECOVERED_TOOL_CALL_SEED), name: name.to_string(), arguments: serde_json::Value::Object(Default::default()), + reasoning: None, }); remaining = after_name; } @@ -3145,4 +3190,32 @@ That's my plan."#; "Text {} middle " ); } + + /// Verify that reasoning normalization strips thinking tags and tool tags + /// from per-tool reasoning, matching the cleaning applied to shared reasoning. + #[test] + fn test_reasoning_normalization_strips_thinking_tags() { + let raw = "Let me consider...Search memory for prior context"; + let pre_truncated = truncate_at_tool_tags(raw); + let cleaned = clean_response(&pre_truncated); + assert!(!cleaned.contains("")); + assert!(cleaned.contains("Search memory")); + } + + #[test] + fn test_reasoning_normalization_strips_tool_tags() { + let raw = "Calling search {\"name\": \"search\"}"; + let pre_truncated = truncate_at_tool_tags(raw); + let cleaned = clean_response(&pre_truncated); + assert!(!cleaned.contains("")); + assert!(cleaned.contains("Calling search")); + } + + #[test] + fn test_reasoning_normalization_empty_after_cleaning() { + let raw = "internal only"; + let pre_truncated = truncate_at_tool_tags(raw); + let cleaned = clean_response(&pre_truncated); + assert!(cleaned.trim().is_empty()); + } } diff --git a/src/llm/rig_adapter.rs b/src/llm/rig_adapter.rs index a9030929..7a6b2ae8 100644 --- a/src/llm/rig_adapter.rs +++ b/src/llm/rig_adapter.rs @@ -490,6 +490,7 @@ fn extract_response( id: tc.id.clone(), name: tc.function.name.clone(), arguments: tc.function.arguments.clone(), + reasoning: None, }); } // Reasoning and Image variants are not mapped to IronClaw types @@ -880,6 +881,7 @@ mod tests { id: "Xt7mK9pQ2".to_string(), name: "search".to_string(), arguments: serde_json::json!({"query": "test"}), + reasoning: None, }; let msg = ChatMessage::assistant_with_tool_calls(Some("thinking".to_string()), vec![tc]); let messages = vec![msg]; @@ -997,6 +999,7 @@ mod tests { id: "".to_string(), name: "search".to_string(), arguments: serde_json::json!({"query": "test"}), + reasoning: None, }; let messages = vec![ChatMessage::assistant_with_tool_calls(None, vec![tc])]; let (_preamble, history) = convert_messages(&messages); @@ -1028,6 +1031,7 @@ mod tests { id: " ".to_string(), name: "search".to_string(), arguments: serde_json::json!({"query": "test"}), + reasoning: None, }; let messages = vec![ChatMessage::assistant_with_tool_calls(None, vec![tc])]; let (_preamble, history) = convert_messages(&messages); @@ -1061,6 +1065,7 @@ mod tests { id: "".to_string(), name: "search".to_string(), arguments: serde_json::json!({"query": "test"}), + reasoning: None, }; let assistant_msg = ChatMessage::assistant_with_tool_calls(None, vec![tc]); let tool_result_msg = ChatMessage { @@ -1380,11 +1385,13 @@ mod tests { id: "call_a".to_string(), name: "search".to_string(), arguments: serde_json::json!({"q": "rust"}), + reasoning: None, }; let tc2 = IronToolCall { id: "call_b".to_string(), name: "fetch".to_string(), arguments: serde_json::json!({"url": "https://example.com"}), + reasoning: None, }; let assistant = ChatMessage::assistant_with_tool_calls(None, vec![tc1, tc2]); let result_a = ChatMessage::tool_result("call_a", "search", "search results"); diff --git a/src/orchestrator/api.rs b/src/orchestrator/api.rs index 37085a8b..8da7ae6f 100644 --- a/src/orchestrator/api.rs +++ b/src/orchestrator/api.rs @@ -14,6 +14,7 @@ use serde::{Deserialize, Serialize}; use tokio::sync::{Mutex, broadcast}; use uuid::Uuid; +use crate::channels::web::types::ToolDecisionDto; use crate::db::Database; use crate::llm::{CompletionRequest, LlmProvider, ToolCompletionRequest}; use crate::orchestrator::auth::{TokenStore, worker_auth_middleware}; @@ -344,6 +345,20 @@ async fn job_event_handler( // gain context/memory tracking capabilities. fallback_deliverable: payload.data.get("fallback_deliverable").cloned(), }, + "reasoning" => { + let narrative = payload + .data + .get("narrative") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + let decisions = ToolDecisionDto::from_json_array(&payload.data["decisions"]); + AppEvent::JobReasoning { + job_id: job_id_str, + narrative, + decisions, + } + } _ => AppEvent::JobStatus { job_id: job_id_str, message: payload diff --git a/src/worker/job.rs b/src/worker/job.rs index 9d5794ca..669c69f0 100644 --- a/src/worker/job.rs +++ b/src/worker/job.rs @@ -18,6 +18,7 @@ use crate::agent::agentic_loop::{ }; use crate::agent::scheduler::WorkerMessage; use crate::agent::task::TaskOutput; +use crate::channels::web::types::ToolDecisionDto; use crate::context::{ContextManager, JobState}; use crate::db::Database; use crate::error::Error; @@ -200,6 +201,19 @@ impl Worker { .map(|s| s.to_string()), fallback_deliverable: data.get("fallback_deliverable").cloned(), }), + "reasoning" => { + let narrative = data + .get("narrative") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + let decisions = ToolDecisionDto::from_json_array(&data["decisions"]); + Some(AppEvent::JobReasoning { + job_id: job_id_str, + narrative, + decisions, + }) + } _ => None, }; if let Some(event) = event { @@ -897,6 +911,11 @@ Report when the job is complete or if you encounter issues you cannot resolve."# id: selection.tool_call_id.clone(), name: selection.tool_name.clone(), arguments: selection.parameters.clone(), + reasoning: if action.reasoning.is_empty() { + None + } else { + Some(action.reasoning.clone()) + }, }], )); @@ -1357,6 +1376,48 @@ impl<'a> LoopDelegate for JobDelegate<'a> { ); } + // Emit reasoning event if any tool calls carry reasoning. + // Sanitize narrative and per-tool rationale through SafetyLayer + // (parity with ChatDelegate in dispatcher.rs). + let sanitized_narrative = content + .as_deref() + .filter(|c| !c.trim().is_empty()) + .map(|c| { + self.worker + .deps + .safety + .sanitize_tool_output("job_narrative", c) + .content + }) + .filter(|c| !c.trim().is_empty()) + .unwrap_or_default(); + let decisions: Vec = tool_calls + .iter() + .filter_map(|tc| { + tc.reasoning.as_ref().map(|r| { + let sanitized = self + .worker + .deps + .safety + .sanitize_tool_output("tool_rationale", r) + .content; + serde_json::json!({ + "tool_name": tc.name, + "rationale": sanitized, + }) + }) + }) + .collect(); + if !decisions.is_empty() { + self.worker.log_event( + "reasoning", + serde_json::json!({ + "narrative": sanitized_narrative, + "decisions": decisions, + }), + ); + } + // Add assistant message with tool_calls (OpenAI protocol) reason_ctx .messages @@ -1371,7 +1432,7 @@ impl<'a> LoopDelegate for JobDelegate<'a> { .map(|tc| ToolSelection { tool_name: tc.name.clone(), parameters: tc.arguments.clone(), - reasoning: String::new(), + reasoning: tc.reasoning.clone().unwrap_or_default(), alternatives: vec![], tool_call_id: tc.id.clone(), }) @@ -1424,6 +1485,11 @@ fn selections_to_tool_calls(selections: &[ToolSelection]) -> Vec { id: s.tool_call_id.clone(), name: s.tool_name.clone(), arguments: s.parameters.clone(), + reasoning: if s.reasoning.is_empty() { + None + } else { + Some(s.reasoning.clone()) + }, }) .collect() } diff --git a/tests/openai_compat_integration.rs b/tests/openai_compat_integration.rs index e1d258ed..b677e57f 100644 --- a/tests/openai_compat_integration.rs +++ b/tests/openai_compat_integration.rs @@ -94,6 +94,7 @@ impl LlmProvider for MockLlmProvider { id: "call_mock_001".to_string(), name: tool.name.clone(), arguments: serde_json::json!({"test": true}), + reasoning: None, }], input_tokens: 15, output_tokens: 8, diff --git a/tests/support/trace_llm.rs b/tests/support/trace_llm.rs index e33caf6b..239cfdb5 100644 --- a/tests/support/trace_llm.rs +++ b/tests/support/trace_llm.rs @@ -566,6 +566,7 @@ impl LlmProvider for TraceLlm { id: tc.id, name: tc.name, arguments: tc.arguments, + reasoning: None, }) .collect(); Ok(ToolCompletionResponse { From 0341fcc9405e3a9f22319891dc1d55d3a67edc06 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 25 Mar 2026 11:45:29 -0700 Subject: [PATCH 5/8] Fix REPL single-message hang and cap CI test duration (#1643) * Fix REPL single-message hang and cap CI test duration * Fix Clippy nested-if lint in REPL startup * Fix single-message approval flow * Handle empty single-message REPL exits * Wait for one-shot event routines before exit --- .github/workflows/test.yml | 24 +++++-- src/agent/agent_loop.rs | 69 ++++++++++++++++-- src/agent/routine_engine.rs | 70 ++++++++++++++++--- src/channels/repl.rs | 60 +++++++++++++--- .../scenarios/test_telegram_hot_activation.py | 4 +- 5 files changed, 196 insertions(+), 31 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 00488c70..5d4eabc0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,6 +12,7 @@ jobs: tests: name: Tests (${{ matrix.name }}) runs-on: ubuntu-latest + timeout-minutes: 45 strategy: fail-fast: false matrix: @@ -40,11 +41,14 @@ jobs: - name: Build WASM channels (for integration tests) run: ./scripts/build-wasm-extensions.sh --channels - name: Run Tests - run: cargo test ${{ matrix.flags }} -- --nocapture + run: | + timeout --signal=INT --kill-after=30s 40m \ + cargo test ${{ matrix.flags }} -- --nocapture heavy-integration-tests: name: Heavy Integration Tests runs-on: ubuntu-latest + timeout-minutes: 20 steps: - name: Checkout repository uses: actions/checkout@v6 @@ -58,9 +62,13 @@ jobs: - name: Build Telegram WASM channel run: cargo build --manifest-path channels-src/telegram/Cargo.toml --target wasm32-wasip2 --release - name: Run thread scheduling integration tests - run: cargo test --no-default-features --features libsql,integration --test e2e_thread_scheduling -- --nocapture + run: | + timeout --signal=INT --kill-after=30s 15m \ + cargo test --no-default-features --features libsql,integration --test e2e_thread_scheduling -- --nocapture - name: Run Telegram thread-scope regression test - run: cargo test --features integration --test telegram_auth_integration test_private_messages_use_chat_id_as_thread_scope -- --exact + run: | + timeout --signal=INT --kill-after=30s 10m \ + cargo test --features integration --test telegram_auth_integration test_private_messages_use_chat_id_as_thread_scope -- --exact telegram-tests: name: Telegram Channel Tests @@ -68,6 +76,7 @@ jobs: github.event_name != 'pull_request' || github.base_ref != 'staging' runs-on: ubuntu-latest + timeout-minutes: 15 steps: - name: Checkout repository uses: actions/checkout@v6 @@ -75,7 +84,9 @@ jobs: uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - name: Run Telegram Channel Tests - run: cargo test --manifest-path channels-src/telegram/Cargo.toml -- --nocapture + run: | + timeout --signal=INT --kill-after=30s 10m \ + cargo test --manifest-path channels-src/telegram/Cargo.toml -- --nocapture windows-build: name: Windows Build (${{ matrix.name }}) @@ -110,6 +121,7 @@ jobs: github.event_name != 'pull_request' || github.base_ref != 'staging' runs-on: ubuntu-latest + timeout-minutes: 30 steps: - name: Checkout repository uses: actions/checkout@v6 @@ -125,7 +137,9 @@ jobs: - name: Build all WASM extensions against current WIT run: ./scripts/build-wasm-extensions.sh - name: Instantiation test (host linker compatibility) - run: cargo test --all-features wit_compat -- --nocapture + run: | + timeout --signal=INT --kill-after=30s 20m \ + cargo test --all-features wit_compat -- --nocapture bench-compile: name: Benchmark Compilation diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs index f51a8db1..e28f11d0 100644 --- a/src/agent/agent_loop.rs +++ b/src/agent/agent_loop.rs @@ -16,6 +16,7 @@ use crate::agent::context_monitor::ContextMonitor; use crate::agent::heartbeat::spawn_heartbeat; use crate::agent::routine_engine::{RoutineEngine, spawn_cron_ticker}; use crate::agent::self_repair::{DefaultSelfRepair, RepairResult, SelfRepair}; +use crate::agent::session::ThreadState; use crate::agent::session_manager::SessionManager; use crate::agent::submission::{Submission, SubmissionParser, SubmissionResult}; use crate::agent::{HeartbeatConfig as AgentHeartbeatConfig, Router, Scheduler, SchedulerDeps}; @@ -84,6 +85,15 @@ fn resolve_owner_scope_notification_user( trimmed_option(explicit_user).or_else(|| trimmed_option(owner_fallback)) } +fn is_single_message_repl(message: &IncomingMessage) -> bool { + message.channel == "repl" + && message + .metadata + .get("single_message_mode") + .and_then(|value| value.as_bool()) + .unwrap_or(false) +} + async fn resolve_channel_notification_user( extension_manager: Option<&Arc>, channel: Option<&str>, @@ -1140,9 +1150,14 @@ impl Agent { && let Submission::UserInput { ref content } = submission && let Some(engine) = self.routine_engine().await { + let single_message_repl = is_single_message_repl(message); // Use post-hook content so that BeforeInbound hooks that rewrite // input are respected by event trigger matching. - let fired = engine.check_event_triggers(message, content).await; + let fired = if single_message_repl { + engine.check_event_triggers_and_wait(message, content).await + } else { + engine.check_event_triggers(message, content).await + }; if fired > 0 { tracing::debug!( channel = %message.channel, @@ -1150,10 +1165,16 @@ impl Agent { fired, "Consumed inbound user message with matching event-triggered routine(s)" ); - return Ok(Some(String::new())); + return if single_message_repl { + Ok(None) + } else { + Ok(Some(String::new())) + }; } } + let session_for_empty_exit = Arc::clone(&session); + // Process based on submission type let result = match submission { Submission::UserInput { content } => { @@ -1263,7 +1284,13 @@ impl Agent { SubmissionResult::Error { message } => { Ok(Some(format!("Error: {}", message))) } - _ => Ok(Some(String::new())), + _ => { + if is_single_message_repl(message) { + Ok(None) + } else { + Ok(Some(String::new())) + } + } }; } // Authorization checks (including restart channel check) are enforced in handle_system_command @@ -1325,7 +1352,26 @@ impl Agent { Ok(Some(content)) } } - SubmissionResult::Ok { message } => Ok(message), + SubmissionResult::Ok { + message: output_message, + } => { + let should_exit = + if output_message.as_deref() == Some("") && is_single_message_repl(message) { + let sess = session_for_empty_exit.lock().await; + sess.threads + .get(&thread_id) + .map(|thread| thread.state != ThreadState::AwaitingApproval) + .unwrap_or(true) + } else { + false + }; + + if should_exit { + Ok(None) + } else { + Ok(output_message) + } + } SubmissionResult::Error { message } => Ok(Some(format!("Error: {}", message))), SubmissionResult::Interrupted => Ok(Some("Interrupted.".into())), SubmissionResult::NeedApproval { .. } => { @@ -1341,7 +1387,7 @@ impl Agent { #[cfg(test)] mod tests { use super::{ - chat_tool_execution_metadata, resolve_routine_notification_user, + chat_tool_execution_metadata, is_single_message_repl, resolve_routine_notification_user, should_fallback_routine_notification, truncate_for_preview, }; use crate::channels::IncomingMessage; @@ -1503,4 +1549,17 @@ mod tests { assert!(should_fallback_routine_notification(&error)); // safety: test-only assertion } + + #[test] + fn single_message_repl_detection_requires_repl_channel_and_metadata_flag() { + let repl = IncomingMessage::new("repl", "owner-scope", "hello") + .with_metadata(serde_json::json!({ "single_message_mode": true })); + let gateway = IncomingMessage::new("gateway", "owner-scope", "hello") + .with_metadata(serde_json::json!({ "single_message_mode": true })); + let plain_repl = IncomingMessage::new("repl", "owner-scope", "hello"); + + assert!(is_single_message_repl(&repl)); // safety: test-only assertion + assert!(!is_single_message_repl(&gateway)); // safety: test-only assertion + assert!(!is_single_message_repl(&plain_repl)); // safety: test-only assertion + } } diff --git a/src/agent/routine_engine.rs b/src/agent/routine_engine.rs index 9c55903f..a3cdb6cd 100644 --- a/src/agent/routine_engine.rs +++ b/src/agent/routine_engine.rs @@ -18,6 +18,7 @@ use std::time::Duration; use chrono::Utc; use regex::Regex; use tokio::sync::{RwLock, mpsc}; +use tokio::task::JoinHandle; use uuid::Uuid; use crate::agent::Scheduler; @@ -45,6 +46,11 @@ enum EventMatcher { System { routine: Routine }, } +struct TriggeredRoutine { + routine: Routine, + detail: String, +} + /// Distinguishes why sandbox is unavailable so error messages are accurate. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SandboxReadiness { @@ -202,6 +208,44 @@ impl RoutineEngine { /// Check incoming message against event triggers. Returns number of routines fired. pub async fn check_event_triggers(&self, message: &IncomingMessage, content: &str) -> usize { + let triggered = self.matching_event_triggers(message, content).await; + let fired = triggered.len(); + for triggered in triggered { + std::mem::drop(self.spawn_fire(triggered.routine, "event", Some(triggered.detail))); + } + fired + } + + /// Fire matching event-triggered routines and wait for them to complete. + /// + /// Used by single-message REPL mode so the process does not exit before + /// background event-triggered routines finish. + pub async fn check_event_triggers_and_wait( + &self, + message: &IncomingMessage, + content: &str, + ) -> usize { + let triggered = self.matching_event_triggers(message, content).await; + let fired = triggered.len(); + let handles: Vec> = triggered + .into_iter() + .map(|triggered| self.spawn_fire(triggered.routine, "event", Some(triggered.detail))) + .collect(); + + for handle in handles { + if let Err(e) = handle.await { + tracing::warn!(error = %e, "Event-triggered routine task failed"); + } + } + + fired + } + + async fn matching_event_triggers( + &self, + message: &IncomingMessage, + content: &str, + ) -> Vec { let cache = self.event_cache.read().await; // Early return if there are no message matchers at all. @@ -209,10 +253,9 @@ impl RoutineEngine { .iter() .any(|m| matches!(m, EventMatcher::Message { .. })) { - return 0; + return Vec::new(); } - - let mut fired = 0; + let mut triggered = Vec::new(); // Collect routine IDs for batch query let routine_ids: Vec = cache @@ -224,13 +267,13 @@ impl RoutineEngine { .collect(); if routine_ids.is_empty() { - return 0; + return Vec::new(); } // Single batch query instead of N queries let concurrent_counts = match self.batch_concurrent_counts(&routine_ids).await { Some(counts) => counts, - None => return 0, + None => return Vec::new(), }; for matcher in cache.iter() { @@ -285,11 +328,13 @@ impl RoutineEngine { } let detail = truncate(content, 200); - self.spawn_fire(routine.clone(), "event", Some(detail)); - fired += 1; + triggered.push(TriggeredRoutine { + routine: routine.clone(), + detail, + }); } - fired + triggered } /// Emit a structured event to system-event routines. @@ -845,7 +890,12 @@ impl RoutineEngine { } /// Spawn a fire in a background task. - fn spawn_fire(&self, routine: Routine, trigger_type: &str, trigger_detail: Option) { + fn spawn_fire( + &self, + routine: Routine, + trigger_type: &str, + trigger_detail: Option, + ) -> JoinHandle<()> { let run = RoutineRun { id: Uuid::new_v4(), routine_id: routine.id, @@ -882,7 +932,7 @@ impl RoutineEngine { return; } execute_routine(engine, routine, run).await; - }); + }) } fn check_cooldown(&self, routine: &Routine) -> bool { diff --git a/src/channels/repl.rs b/src/channels/repl.rs index 61c68d13..41d73a8c 100644 --- a/src/channels/repl.rs +++ b/src/channels/repl.rs @@ -431,6 +431,18 @@ impl ReplChannel { let _ = execute!(stderr, terminal::Clear(terminal::ClearType::FromCursorDown)); } } + + async fn finish_single_message_turn(&self) { + if self.single_message.is_none() { + return; + } + + let tx = self.msg_tx.lock().ok().and_then(|mut guard| guard.take()); + if let Some(tx) = tx { + let msg = IncomingMessage::new("repl", &self.user_id, "/quit"); + let _ = tx.send(msg).await; + } + } } impl Default for ReplChannel { @@ -480,7 +492,9 @@ impl Channel for ReplChannel { async fn start(&self) -> Result { let (tx, rx) = mpsc::channel(32); - // Store tx so send_status can inject approval responses directly + // Approval prompts inject responses back through this sender. + // In single-message mode we keep it until the turn finishes, then + // drop it after enqueuing /quit so the receiver stream can close. if let Ok(mut guard) = self.msg_tx.lock() { *guard = Some(tx.clone()); } @@ -496,11 +510,10 @@ impl Channel for ReplChannel { // Single message mode: send it and return if let Some(msg) = single_message { - let incoming = IncomingMessage::new("repl", &user_id, &msg).with_timezone(&sys_tz); + let incoming = IncomingMessage::new("repl", &user_id, &msg) + .with_metadata(serde_json::json!({ "single_message_mode": true })) + .with_timezone(&sys_tz); let _ = tx.blocking_send(incoming); - // Ensure the agent exits after handling exactly one turn in -m mode, - // even when other channels (gateway/http) are enabled. - let _ = tx.blocking_send(IncomingMessage::new("repl", &user_id, "/quit")); return; } @@ -663,6 +676,7 @@ impl Channel for ReplChannel { println!(); println!(); self.stdin_locked.store(false, Ordering::Relaxed); + self.finish_single_message_turn().await; return Ok(()); } @@ -681,6 +695,7 @@ impl Channel for ReplChannel { println!(); // Unlock stdin so readline can resume self.stdin_locked.store(false, Ordering::Relaxed); + self.finish_single_message_turn().await; Ok(()) } @@ -780,6 +795,7 @@ impl Channel for ReplChannel { let msg_tx = Arc::clone(&self.msg_tx); let user_id = self.user_id.clone(); let lock_flag = Arc::clone(&self.stdin_locked); + let single_message_mode = self.single_message.is_some(); tokio::task::spawn_blocking(move || { let action = run_approval_selector(allow_always).unwrap_or("n"); // Unlock stdin so readline can resume after approval @@ -788,7 +804,12 @@ impl Channel for ReplChannel { return; }; if let Some(tx) = guard.as_ref() { - let msg = IncomingMessage::new("repl", &user_id, action); + let msg = if single_message_mode { + IncomingMessage::new("repl", &user_id, action) + .with_metadata(serde_json::json!({ "single_message_mode": true })) + } else { + IncomingMessage::new("repl", &user_id, action) + }; let _ = tx.blocking_send(msg); } }); @@ -889,6 +910,7 @@ impl Channel for ReplChannel { #[cfg(test)] mod tests { use futures::StreamExt; + use tokio::time::{Duration, timeout}; use super::*; @@ -897,16 +919,36 @@ mod tests { let repl = ReplChannel::with_message("hi".to_string()); let mut stream = repl.start().await.expect("repl start should succeed"); - let first = stream.next().await.expect("first message missing"); + let first = timeout(Duration::from_secs(1), stream.next()) + .await + .expect("timed out waiting for first message") + .expect("first message missing"); assert_eq!(first.channel, "repl"); assert_eq!(first.content, "hi"); - let second = stream.next().await.expect("quit message missing"); + assert!( + timeout(Duration::from_millis(100), stream.next()) + .await + .is_err(), + "single-message mode should wait for the turn to finish before quitting" + ); + + repl.respond(&first, OutgoingResponse::text("done")) + .await + .expect("respond should succeed"); + + let second = timeout(Duration::from_secs(1), stream.next()) + .await + .expect("timed out waiting for quit message") + .expect("quit message missing"); assert_eq!(second.channel, "repl"); assert_eq!(second.content, "/quit"); assert!( - stream.next().await.is_none(), + timeout(Duration::from_secs(1), stream.next()) + .await + .expect("timed out waiting for stream to close") + .is_none(), "stream should end after /quit" ); } diff --git a/tests/e2e/scenarios/test_telegram_hot_activation.py b/tests/e2e/scenarios/test_telegram_hot_activation.py index 261b837e..fede2be5 100644 --- a/tests/e2e/scenarios/test_telegram_hot_activation.py +++ b/tests/e2e/scenarios/test_telegram_hot_activation.py @@ -253,6 +253,6 @@ async def test_telegram_hot_activation_transitions_installed_to_active(page): assert await card.locator(SEL["ext_pairing_label"]).count() == 0 assert captured_setup_payloads == [ - {"secrets": {"telegram_bot_token": "123456789:ABCdefGhI"}}, - {"secrets": {}}, + {"secrets": {"telegram_bot_token": "123456789:ABCdefGhI"}, "fields": {}}, + {"secrets": {}, "fields": {}}, ] From c949521d8d153ecb3af30877779f8c160278ca09 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 25 Mar 2026 13:17:32 -0700 Subject: [PATCH 6/8] Fix MCP lifecycle trace user scope (#1646) * Fix REPL single-message hang and cap CI test duration * Fix Clippy nested-if lint in REPL startup * Fix single-message approval flow * Handle empty single-message REPL exits * Wait for one-shot event routines before exit * Fix MCP lifecycle trace user scope --- tests/e2e_advanced_traces.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/e2e_advanced_traces.rs b/tests/e2e_advanced_traces.rs index b3efc8d9..ce18ad3d 100644 --- a/tests/e2e_advanced_traces.rs +++ b/tests/e2e_advanced_traces.rs @@ -587,6 +587,7 @@ mod advanced { async fn mcp_extension_lifecycle() { use crate::support::mock_mcp_server::{MockToolResponse, start_mock_mcp_server}; use ironclaw::extensions::{AuthHint, ExtensionKind, ExtensionSource, RegistryEntry}; + const TEST_USER_ID: &str = "test-user"; // 1. Start mock MCP server with pre-configured tool responses. let mock_server = start_mock_mcp_server(vec![ @@ -654,14 +655,14 @@ mod advanced { ext_mgr .secrets() .create( - "default", + TEST_USER_ID, ironclaw::secrets::CreateSecretParams::new(secret_name, "mock-access-token") .with_provider("mcp:mock-notion".to_string()), ) .await .expect("failed to inject test token"); - let activate_result = ext_mgr.activate("mock-notion", "default").await; + let activate_result = ext_mgr.activate("mock-notion", TEST_USER_ID).await; assert!( activate_result.is_ok(), "activation failed: {:?}", From ab0ad948f36c7cc88b1aecf2e92dd0ff94569a94 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 25 Mar 2026 13:47:12 -0700 Subject: [PATCH 7/8] Normalize cron schedules on routine create (#1648) * Fix REPL single-message hang and cap CI test duration * Fix Clippy nested-if lint in REPL startup * Fix single-message approval flow * Handle empty single-message REPL exits * Wait for one-shot event routines before exit * Fix MCP lifecycle trace user scope * Normalize cron schedules on routine create --- src/tools/builtin/routine.rs | 16 +++++++++++++++- tests/e2e_builtin_tool_coverage.rs | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/tools/builtin/routine.rs b/src/tools/builtin/routine.rs index f4313483..bbc24139 100644 --- a/src/tools/builtin/routine.rs +++ b/src/tools/builtin/routine.rs @@ -915,7 +915,7 @@ fn parse_routine_create_request( fn build_routine_trigger(trigger: &NormalizedTriggerRequest) -> Trigger { match trigger { NormalizedTriggerRequest::Cron { schedule, timezone } => Trigger::Cron { - schedule: schedule.clone(), + schedule: normalize_cron_expression(schedule), timezone: timezone.clone(), }, NormalizedTriggerRequest::Manual => Trigger::Manual, @@ -1836,6 +1836,20 @@ mod tests { assert_eq!(parsed.cooldown_secs, 30); } + #[test] + fn build_routine_trigger_normalizes_cron_schedule() { + let trigger = build_routine_trigger(&NormalizedTriggerRequest::Cron { + schedule: "0 0 9 * * MON-FRI".to_string(), + timezone: Some("UTC".to_string()), + }); + + assert!(matches!( + trigger, + Trigger::Cron { schedule, timezone } + if schedule == "0 0 9 * * MON-FRI *" && timezone.as_deref() == Some("UTC") + )); + } + #[test] fn parses_grouped_message_event_with_tools() { let params = serde_json::json!({ diff --git a/tests/e2e_builtin_tool_coverage.rs b/tests/e2e_builtin_tool_coverage.rs index 42d7fb75..1c3cc6a2 100644 --- a/tests/e2e_builtin_tool_coverage.rs +++ b/tests/e2e_builtin_tool_coverage.rs @@ -439,7 +439,7 @@ mod tests { match &routine.trigger { Trigger::Cron { schedule, timezone } => { - assert_eq!(schedule, "0 0 9 * * MON-FRI"); + assert_eq!(schedule, "0 0 9 * * MON-FRI *"); assert_eq!(timezone.as_deref(), Some("UTC")); } other => panic!("expected cron trigger, got {other:?}"), From 86d11430640da22d8f890bb9b2df867dda1e668e Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 25 Mar 2026 14:36:53 -0700 Subject: [PATCH 8/8] Fix libsql prompt scope regressions (#1651) --- src/agent/dispatcher.rs | 7 +++- src/workspace/mod.rs | 55 +++++++++++++++++++++++++++++ src/workspace/repository.rs | 1 + tests/e2e_workspace_coverage.rs | 4 ++- tests/multi_tenant_system_prompt.rs | 14 ++++---- 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/src/agent/dispatcher.rs b/src/agent/dispatcher.rs index cba84c35..fe208c1b 100644 --- a/src/agent/dispatcher.rs +++ b/src/agent/dispatcher.rs @@ -63,7 +63,12 @@ impl Agent { ); let system_prompt = if let Some(ws) = self.workspace() { - match ws + let scoped_workspace = if ws.user_id() == message.user_id { + Arc::clone(ws) + } else { + Arc::new(ws.scoped_to_user(&message.user_id)) + }; + match scoped_workspace .system_prompt_for_context_tz(is_group_chat, user_tz) .await { diff --git a/src/workspace/mod.rs b/src/workspace/mod.rs index 0242047f..51d7d2fc 100644 --- a/src/workspace/mod.rs +++ b/src/workspace/mod.rs @@ -149,6 +149,7 @@ fn reject_if_injected(path: &str, content: &str) -> Result<(), WorkspaceError> { /// /// Allows Workspace to work with either a PostgreSQL `Repository` (the original /// path) or any `Database` trait implementation (e.g. libSQL backend). +#[derive(Clone)] enum WorkspaceStorage { /// PostgreSQL-backed repository (uses connection pool directly). #[cfg(feature = "postgres")] @@ -576,6 +577,60 @@ impl Workspace { self } + /// Clone the workspace configuration for a different primary user scope. + /// + /// This preserves search config, embeddings, shared read scopes, memory + /// layers, and privacy classifier while switching the primary read/write + /// scope to `user_id`. + pub fn scoped_to_user(&self, user_id: impl Into) -> Self { + let user_id = user_id.into(); + + let mut memory_layers = self.memory_layers.clone(); + for layer in &mut memory_layers { + if layer.sensitivity == crate::workspace::layer::LayerSensitivity::Private + && layer.scope == self.user_id + { + layer.scope = user_id.clone(); + } + } + + let mut read_user_ids = vec![user_id.clone()]; + for scope in &self.read_user_ids { + if scope != &self.user_id && !read_user_ids.contains(scope) { + read_user_ids.push(scope.clone()); + } + } + for scope in crate::workspace::layer::MemoryLayer::read_scopes(&memory_layers) { + if !read_user_ids.contains(&scope) { + read_user_ids.push(scope); + } + } + + let preserve_flags = user_id == self.user_id; + Self { + user_id, + read_user_ids, + agent_id: self.agent_id, + storage: self.storage.clone(), + embeddings: self.embeddings.clone(), + bootstrap_pending: std::sync::atomic::AtomicBool::new(if preserve_flags { + self.bootstrap_pending + .load(std::sync::atomic::Ordering::Acquire) + } else { + false + }), + bootstrap_completed: std::sync::atomic::AtomicBool::new(if preserve_flags { + self.bootstrap_completed + .load(std::sync::atomic::Ordering::Acquire) + } else { + false + }), + search_defaults: self.search_defaults.clone(), + memory_layers, + privacy_classifier: self.privacy_classifier.clone(), + } + } + /// Get the user ID (primary scope for writes). pub fn user_id(&self) -> &str { &self.user_id diff --git a/src/workspace/repository.rs b/src/workspace/repository.rs index 78ddfec5..13f6816b 100644 --- a/src/workspace/repository.rs +++ b/src/workspace/repository.rs @@ -15,6 +15,7 @@ use crate::workspace::document::{MemoryChunk, MemoryDocument, WorkspaceEntry}; use crate::workspace::search::{RankedResult, SearchConfig, SearchResult, fuse_results}; /// Database repository for workspace operations. +#[derive(Clone)] pub struct Repository { pool: Pool, } diff --git a/tests/e2e_workspace_coverage.rs b/tests/e2e_workspace_coverage.rs index 396b676e..68956d30 100644 --- a/tests/e2e_workspace_coverage.rs +++ b/tests/e2e_workspace_coverage.rs @@ -12,6 +12,7 @@ mod tests { use crate::support::test_rig::TestRigBuilder; use crate::support::trace_llm::LlmTrace; + use ironclaw::workspace::Workspace; // ----------------------------------------------------------------------- // Test 1: write_chunk_search @@ -268,6 +269,7 @@ mod tests { #[tokio::test] async fn identity_in_system_prompt() { + const TEST_USER_ID: &str = "test-user"; let trace = LlmTrace::from_file(concat!( env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/llm_traces/workspace/identity_prompt.json" @@ -280,7 +282,7 @@ mod tests { .await; // Seed an IDENTITY.md so the system prompt has real content to inject. - let ws = rig.workspace().expect("workspace must be available"); + let ws = Workspace::new_with_db(TEST_USER_ID, rig.database().clone()); ws.write( "IDENTITY.md", "I am TestBot, a helpful testing assistant created for E2E verification.", diff --git a/tests/multi_tenant_system_prompt.rs b/tests/multi_tenant_system_prompt.rs index ece794bf..b89e6cb5 100644 --- a/tests/multi_tenant_system_prompt.rs +++ b/tests/multi_tenant_system_prompt.rs @@ -1,10 +1,10 @@ -//! Tests proving that multi-tenant system prompts are broken. +//! Regression tests for multi-tenant system prompts. //! -//! Bug: In multi-tenant mode, the agent loop uses `self.workspace()` which -//! returns a single shared workspace (user_id="default"). Identity files -//! (IDENTITY.md, SOUL.md, USER.md) seeded under per-user IDs ("alice", -//! "bob") are invisible to this workspace, so the system prompt is -//! empty/wrong. +//! The agent must build the conversational system prompt from a workspace +//! scoped to the incoming message's user, not from the shared owner-scope +//! workspace created at startup. Otherwise per-user identity files +//! (IDENTITY.md, SOUL.md, USER.md) become invisible and different users can +//! see the same owner-scoped prompt. //! //! These tests: //! 1. Seed identity files for two users (alice, bob) in the database @@ -13,7 +13,7 @@ //! correct user's identity //! 4. Verify user A's identity doesn't leak into user B's prompt //! -//! All tests are expected to FAIL until the bug is fixed. +//! These tests ensure each user's identity is isolated correctly. #[cfg(feature = "libsql")] mod support;