From 31eb833cdfbbb2c5c538e87e2f72407a645ddcef Mon Sep 17 00:00:00 2001 From: jaberjaber23 Date: Tue, 12 May 2026 14:34:22 +0300 Subject: [PATCH] agent history --- crates/openfang-kernel/src/heartbeat.rs | 1 + crates/openfang-kernel/src/kernel.rs | 5 + crates/openfang-kernel/src/registry.rs | 1 + crates/openfang-kernel/src/wizard.rs | 1 + crates/openfang-runtime/src/agent_loop.rs | 211 ++++++++++++++++------ crates/openfang-types/src/agent.rs | 25 +++ docs/agent-templates.md | 7 + 7 files changed, 194 insertions(+), 57 deletions(-) diff --git a/crates/openfang-kernel/src/heartbeat.rs b/crates/openfang-kernel/src/heartbeat.rs index 60e82fb4..da0eb803 100644 --- a/crates/openfang-kernel/src/heartbeat.rs +++ b/crates/openfang-kernel/src/heartbeat.rs @@ -344,6 +344,7 @@ mod tests { tool_allowlist: vec![], tool_blocklist: vec![], cache_context: false, + max_history_messages: None, }, state, mode: AgentMode::default(), diff --git a/crates/openfang-kernel/src/kernel.rs b/crates/openfang-kernel/src/kernel.rs index c23799ad..a09fc153 100644 --- a/crates/openfang-kernel/src/kernel.rs +++ b/crates/openfang-kernel/src/kernel.rs @@ -7546,6 +7546,7 @@ mod tests { tool_allowlist: vec![], tool_blocklist: vec![], cache_context: false, + max_history_messages: None, }; manifest.capabilities.tools = vec!["file_read".to_string(), "web_fetch".to_string()]; manifest.capabilities.agent_spawn = true; @@ -7588,6 +7589,7 @@ mod tests { tool_allowlist: vec![], tool_blocklist: vec![], cache_context: false, + max_history_messages: None, }; let mut disk = entry.clone(); disk.description = "new".to_string(); @@ -7638,6 +7640,7 @@ mod tests { tool_allowlist: vec![], tool_blocklist: vec![], cache_context: false, + max_history_messages: None, }; let mut disk = entry.clone(); disk.workspace = Some(std::path::PathBuf::from("/new")); @@ -7693,6 +7696,7 @@ mod tests { tool_allowlist: vec![], tool_blocklist: vec![], cache_context: false, + max_history_messages: None, }; // Current kernel config now says mode = Full. @@ -7805,6 +7809,7 @@ mod tests { tool_allowlist: vec![], tool_blocklist: vec![], cache_context: false, + max_history_messages: None, } } diff --git a/crates/openfang-kernel/src/registry.rs b/crates/openfang-kernel/src/registry.rs index aa00f476..76501d59 100644 --- a/crates/openfang-kernel/src/registry.rs +++ b/crates/openfang-kernel/src/registry.rs @@ -396,6 +396,7 @@ mod tests { tool_allowlist: vec![], tool_blocklist: vec![], cache_context: false, + max_history_messages: None, }, state: AgentState::Created, mode: AgentMode::default(), diff --git a/crates/openfang-kernel/src/wizard.rs b/crates/openfang-kernel/src/wizard.rs index 6aa766c1..bafd5291 100644 --- a/crates/openfang-kernel/src/wizard.rs +++ b/crates/openfang-kernel/src/wizard.rs @@ -183,6 +183,7 @@ impl SetupWizard { tool_allowlist: vec![], tool_blocklist: vec![], cache_context: false, + max_history_messages: None, }; let skills_to_install: Vec = intent diff --git a/crates/openfang-runtime/src/agent_loop.rs b/crates/openfang-runtime/src/agent_loop.rs index 0add089e..92a2bc38 100644 --- a/crates/openfang-runtime/src/agent_loop.rs +++ b/crates/openfang-runtime/src/agent_loop.rs @@ -40,21 +40,44 @@ const MAX_RETRIES: u32 = 3; /// Base delay for exponential backoff (milliseconds). const BASE_RETRY_DELAY_MS: u64 = 1000; -/// Timeout for individual tool executions (seconds). +/// Default timeout for individual tool executions (seconds). /// Raised from 60s to 120s for browser automation and long-running builds. +/// Overridable via `OPENFANG_TOOL_TIMEOUT_SECS` env var. Set to `0` to disable +/// the timeout entirely (useful for slow local inference like vLLM on old GPUs). const TOOL_TIMEOUT_SECS: u64 = 120; -/// Timeout for inter-agent tool calls (seconds). +/// Default timeout for inter-agent tool calls (seconds). /// Agent delegation (agent_send, agent_spawn) can involve a full agent loop on the /// target, so these need a significantly longer timeout than regular tools. +/// Overridable via `OPENFANG_AGENT_TOOL_TIMEOUT_SECS` env var. Set to `0` to +/// disable (issue #1125: slow vLLM rigs running Hands need unbounded waits). const AGENT_TOOL_TIMEOUT_SECS: u64 = 600; +/// Parse a u64 env var, returning `None` when unset or unparseable so the +/// caller falls back to the compiled-in default. +fn env_timeout_secs(var: &str) -> Option { + std::env::var(var).ok().and_then(|s| s.trim().parse().ok()) +} + /// Returns the appropriate timeout duration for a given tool name. /// Inter-agent calls get a longer timeout since they may trigger full agent loops. -fn tool_timeout_for(tool_name: &str) -> Duration { - match tool_name { - "agent_send" | "agent_spawn" => Duration::from_secs(AGENT_TOOL_TIMEOUT_SECS), - _ => Duration::from_secs(TOOL_TIMEOUT_SECS), +/// +/// Returns `None` when the operator opted out by setting the relevant env var +/// to `0`. In that case the tool runs with no upper bound, which is what users +/// on slow local inference (vLLM on old GPUs) want for Hands and inter-agent +/// delegation (issue #1125). +fn tool_timeout_for(tool_name: &str) -> Option { + let secs = match tool_name { + "agent_send" | "agent_spawn" => { + env_timeout_secs("OPENFANG_AGENT_TOOL_TIMEOUT_SECS") + .unwrap_or(AGENT_TOOL_TIMEOUT_SECS) + } + _ => env_timeout_secs("OPENFANG_TOOL_TIMEOUT_SECS").unwrap_or(TOOL_TIMEOUT_SECS), + }; + if secs == 0 { + None + } else { + Some(Duration::from_secs(secs)) } } @@ -62,8 +85,9 @@ fn tool_timeout_for(tool_name: &str) -> Duration { /// Raised from 3 to 5 to allow longer-form generation. const MAX_CONTINUATIONS: u32 = 5; -/// Maximum message history size before auto-trimming to prevent context overflow. -const MAX_HISTORY_MESSAGES: usize = 20; +/// Default maximum message history size before auto-trimming to prevent context overflow. +/// Per-agent overrides come from `AgentManifest::max_history_messages` (issue #871). +const MAX_HISTORY_MESSAGES: usize = openfang_types::agent::DEFAULT_MAX_HISTORY_MESSAGES; /// Detect when the LLM claims to have performed an action (sent, posted, emailed) /// without actually calling any tools. Prevents hallucinated completions. @@ -426,12 +450,15 @@ pub async fn run_agent_loop( // Safety valve: trim excessively long message histories to prevent context overflow. // The full compaction system handles sophisticated summarization, but this prevents // the catastrophic case where 200+ messages cause instant context overflow. - if messages.len() > MAX_HISTORY_MESSAGES { - let trim_count = messages.len() - MAX_HISTORY_MESSAGES; + // Per-agent cap: manifest override -> runtime default (issue #871). + let max_history = manifest.effective_max_history_messages(); + if messages.len() > max_history { + let trim_count = messages.len() - max_history; warn!( agent = %manifest.name, total_messages = messages.len(), trimming = trim_count, + max_history = max_history, "Trimming old messages to prevent context overflow" ); messages.drain(..trim_count); @@ -886,49 +913,51 @@ pub async fn run_agent_loop( // Resolve effective exec policy (per-agent override or global) let effective_exec_policy = manifest.exec_policy.as_ref(); - // Timeout-wrapped execution - let timeout = tool_timeout_for(&tool_call.name); - let timeout_secs = timeout.as_secs(); - let result = match tokio::time::timeout( - timeout, - tool_runner::execute_tool( - &tool_call.id, - &tool_call.name, - &tool_call.input, - kernel.as_ref(), - Some(&allowed_tool_names), - Some(&caller_id_str), - skill_registry, - mcp_connections, - web_ctx, - browser_ctx, - if hand_allowed_env.is_empty() { - None - } else { - Some(&hand_allowed_env) - }, - workspace_root, - media_engine, - effective_exec_policy, - tts_engine, - docker_config, - process_manager, - ), - ) - .await - { - Ok(result) => result, - Err(_) => { - warn!(tool = %tool_call.name, "Tool execution timed out after {}s", timeout_secs); - openfang_types::tool::ToolResult { - tool_use_id: tool_call.id.clone(), - content: format!( - "Tool '{}' timed out after {}s.", - tool_call.name, timeout_secs - ), - is_error: true, + // Timeout-wrapped execution. `tool_timeout_for` returns None + // when the operator disabled the timeout (issue #1125). + let timeout_opt = tool_timeout_for(&tool_call.name); + let exec_fut = tool_runner::execute_tool( + &tool_call.id, + &tool_call.name, + &tool_call.input, + kernel.as_ref(), + Some(&allowed_tool_names), + Some(&caller_id_str), + skill_registry, + mcp_connections, + web_ctx, + browser_ctx, + if hand_allowed_env.is_empty() { + None + } else { + Some(&hand_allowed_env) + }, + workspace_root, + media_engine, + effective_exec_policy, + tts_engine, + docker_config, + process_manager, + ); + let result = match timeout_opt { + Some(timeout) => { + let timeout_secs = timeout.as_secs(); + match tokio::time::timeout(timeout, exec_fut).await { + Ok(result) => result, + Err(_) => { + warn!(tool = %tool_call.name, "Tool execution timed out after {}s", timeout_secs); + openfang_types::tool::ToolResult { + tool_use_id: tool_call.id.clone(), + content: format!( + "Tool '{}' timed out after {}s.", + tool_call.name, timeout_secs + ), + is_error: true, + } + } } } + None => exec_fut.await, }; // Fire AfterToolCall hook @@ -1031,7 +1060,12 @@ pub async fn run_agent_loop( } else { text }; - session.messages.push(Message::assistant(&text)); + // Issue #1148: preserve Thinking / RedactedThinking blocks + // present in the response so reasoning state survives + // MaxTokens truncation — same as the EndTurn branch. + let assistant_msg = + build_assistant_message_preserving_thinking(&response.content, &text); + session.messages.push(assistant_msg); if let Err(e) = memory.save_session_async(session).await { warn!("Failed to save session on max continuations: {e}"); } @@ -1062,10 +1096,15 @@ pub async fn run_agent_loop( directives: Default::default(), }); } - // Model hit token limit — add partial response and continue + // Model hit token limit — add partial response and continue. + // Issue #1148: preserve full response content (Thinking, + // RedactedThinking, etc.) so reasoning state is not dropped + // when continuing across the token-limit boundary. let text = response.text(); - session.messages.push(Message::assistant(&text)); - messages.push(Message::assistant(&text)); + let assistant_msg = + build_assistant_message_preserving_thinking(&response.content, &text); + session.messages.push(assistant_msg.clone()); + messages.push(assistant_msg); session.messages.push(Message::user("Please continue.")); messages.push(Message::user("Please continue.")); warn!(iteration, "Max tokens hit, continuing"); @@ -1627,12 +1666,15 @@ pub async fn run_agent_loop_streaming( let mut accumulated_text = String::new(); // Safety valve: trim excessively long message histories to prevent context overflow. - if messages.len() > MAX_HISTORY_MESSAGES { - let trim_count = messages.len() - MAX_HISTORY_MESSAGES; + // Per-agent cap: manifest override -> runtime default (issue #871). + let max_history = manifest.effective_max_history_messages(); + if messages.len() > max_history { + let trim_count = messages.len() - max_history; warn!( agent = %manifest.name, total_messages = messages.len(), trimming = trim_count, + max_history = max_history, "Trimming old messages to prevent context overflow (streaming)" ); messages.drain(..trim_count); @@ -3374,6 +3416,61 @@ mod tests { #[test] fn test_max_history_messages() { assert_eq!(MAX_HISTORY_MESSAGES, 20); + assert_eq!( + openfang_types::agent::DEFAULT_MAX_HISTORY_MESSAGES, + MAX_HISTORY_MESSAGES + ); + } + + /// Issue #871: an agent with a manifest override uses that value. + #[test] + fn test_effective_max_history_uses_manifest_override() { + let mut manifest = openfang_types::agent::AgentManifest::default(); + manifest.max_history_messages = Some(40); + assert_eq!(manifest.effective_max_history_messages(), 40); + + manifest.max_history_messages = Some(6); + assert_eq!(manifest.effective_max_history_messages(), 6); + } + + /// Issue #871: an agent without an override falls back to the runtime + /// default. `Some(0)` is also treated as the default to avoid an agent + /// accidentally disabling history entirely. + #[test] + fn test_effective_max_history_falls_back_to_default() { + let mut manifest = openfang_types::agent::AgentManifest::default(); + manifest.max_history_messages = None; + assert_eq!( + manifest.effective_max_history_messages(), + MAX_HISTORY_MESSAGES + ); + + manifest.max_history_messages = Some(0); + assert_eq!( + manifest.effective_max_history_messages(), + MAX_HISTORY_MESSAGES + ); + } + + /// Issue #871: `max_history_messages` round-trips through serde with + /// `#[serde(default)]`, so manifests without the field still deserialize. + #[test] + fn test_manifest_max_history_round_trip_json() { + let json_no_override = r#"{"name":"worker","module":"builtin:chat"}"#; + let manifest: openfang_types::agent::AgentManifest = + serde_json::from_str(json_no_override).unwrap(); + assert_eq!(manifest.max_history_messages, None); + assert_eq!( + manifest.effective_max_history_messages(), + MAX_HISTORY_MESSAGES + ); + + let json_with_override = + r#"{"name":"orchestrator","module":"builtin:chat","max_history_messages":40}"#; + let manifest: openfang_types::agent::AgentManifest = + serde_json::from_str(json_with_override).unwrap(); + assert_eq!(manifest.max_history_messages, Some(40)); + assert_eq!(manifest.effective_max_history_messages(), 40); } fn sample_image_block() -> ContentBlock { diff --git a/crates/openfang-types/src/agent.rs b/crates/openfang-types/src/agent.rs index c3ebbf67..8f509b25 100644 --- a/crates/openfang-types/src/agent.rs +++ b/crates/openfang-types/src/agent.rs @@ -497,6 +497,29 @@ pub struct AgentManifest { /// on the next message. See issue #843. #[serde(default)] pub cache_context: bool, + /// Per-agent override for the maximum number of in-context message turns + /// kept before the safety-valve trim kicks in. `None` falls back to the + /// runtime default (20). Primary/orchestrator agents typically want more + /// (e.g. 40); short-lived worker agents want less (e.g. 4-6) so their + /// `agent_send` results stay focused. See issue #871. + #[serde(default)] + pub max_history_messages: Option, +} + +/// Runtime default for `AgentManifest::max_history_messages` when the agent +/// does not specify an override. Kept in sync with `agent_loop::MAX_HISTORY_MESSAGES`. +pub const DEFAULT_MAX_HISTORY_MESSAGES: usize = 20; + +impl AgentManifest { + /// Resolve the effective history cap: agent manifest override, falling + /// back to `DEFAULT_MAX_HISTORY_MESSAGES`. Values of `Some(0)` are treated + /// as the default to avoid an agent accidentally disabling history. + pub fn effective_max_history_messages(&self) -> usize { + match self.max_history_messages { + Some(n) if n > 0 => n, + _ => DEFAULT_MAX_HISTORY_MESSAGES, + } + } } fn default_true() -> bool { @@ -532,6 +555,7 @@ impl Default for AgentManifest { tool_allowlist: Vec::new(), tool_blocklist: Vec::new(), cache_context: false, + max_history_messages: None, } } } @@ -790,6 +814,7 @@ mod tests { tool_allowlist: Vec::new(), tool_blocklist: Vec::new(), cache_context: false, + max_history_messages: None, }; let json = serde_json::to_string(&manifest).unwrap(); let deserialized: AgentManifest = serde_json::from_str(&json).unwrap(); diff --git a/docs/agent-templates.md b/docs/agent-templates.md index 51bdd5d7..d7fcf28a 100644 --- a/docs/agent-templates.md +++ b/docs/agent-templates.md @@ -815,6 +815,13 @@ module = "builtin:chat" # Optional metadata tags = ["tag1", "tag2"] +# Optional per-agent history cap (issue #871). Falls back to the runtime +# default (20) when unset. Bump it for primary/orchestrator agents that need +# longer conversational memory, lower it for short-lived worker agents so +# their agent_send results stay focused. +# max_history_messages = 40 # primary/orchestrator +# max_history_messages = 6 # worker + # Model configuration (required) [model] provider = "gemini" # Provider: gemini, deepseek, groq, openai, anthropic, etc.