fix(runtime,kernel): HandTool 空壳修复 — 桥接到 HandRegistry 真实执行
Some checks failed
CI / Lint & TypeCheck (push) Has been cancelled
CI / Unit Tests (push) Has been cancelled
CI / Build Frontend (push) Has been cancelled
CI / Rust Check (push) Has been cancelled
CI / Security Scan (push) Has been cancelled
CI / E2E Tests (push) Has been cancelled
Some checks failed
CI / Lint & TypeCheck (push) Has been cancelled
CI / Unit Tests (push) Has been cancelled
CI / Build Frontend (push) Has been cancelled
CI / Rust Check (push) Has been cancelled
CI / Security Scan (push) Has been cancelled
CI / E2E Tests (push) Has been cancelled
B-HAND-1 修复: LLM 调用 hand_quiz/hand_researcher 等 Hand 工具后,
HandTool::execute() 原来返回假成功 JSON, 实际 Hand 并不执行.
修复方案 (沿用 SkillExecutor 模式):
- tool.rs: 新增 HandExecutor trait + ToolContext.hand_executor 字段
- hand_tool.rs: execute() 通过 context.hand_executor 分发到真实执行
- loop_runner.rs: AgentLoop 新增 hand_executor 字段 + builder + 3处 ToolContext 传递
- adapters.rs: 新增 KernelHandExecutor 桥接 HandRegistry.execute()
- kernel/mod.rs: 初始化 KernelHandExecutor + 注册到 AgentLoop
- messaging.rs: 两处 AgentLoop 构建添加 .with_hand_executor()
数据流: LLM tool call → HandTool::execute() → ToolContext.hand_executor
→ KernelHandExecutor → HandRegistry.execute() → Hand trait impl
809 tests passed, 0 failed.
This commit is contained in:
@@ -3,11 +3,12 @@
|
|||||||
use std::pin::Pin;
|
use std::pin::Pin;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use async_trait::async_trait;
|
use async_trait::async_trait;
|
||||||
use serde_json::Value;
|
use serde_json::{json, Value};
|
||||||
|
|
||||||
use zclaw_runtime::{LlmDriver, tool::SkillExecutor};
|
use zclaw_runtime::{LlmDriver, tool::{SkillExecutor, HandExecutor}};
|
||||||
use zclaw_skills::{SkillRegistry, LlmCompleter};
|
use zclaw_skills::{SkillRegistry, LlmCompleter};
|
||||||
use zclaw_types::Result;
|
use zclaw_hands::HandRegistry;
|
||||||
|
use zclaw_types::{AgentId, Result};
|
||||||
|
|
||||||
/// Adapter that bridges `zclaw_runtime::LlmDriver` -> `zclaw_skills::LlmCompleter`
|
/// Adapter that bridges `zclaw_runtime::LlmDriver` -> `zclaw_skills::LlmCompleter`
|
||||||
pub(crate) struct LlmDriverAdapter {
|
pub(crate) struct LlmDriverAdapter {
|
||||||
@@ -134,3 +135,47 @@ impl AgentInbox {
|
|||||||
self.pending.push_back(envelope);
|
self.pending.push_back(envelope);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Hand executor implementation for Kernel
|
||||||
|
///
|
||||||
|
/// Bridges `zclaw_runtime::tool::HandExecutor` → `zclaw_hands::HandRegistry`,
|
||||||
|
/// allowing `HandTool::execute()` to dispatch to the real Hand implementations.
|
||||||
|
pub struct KernelHandExecutor {
|
||||||
|
hands: Arc<HandRegistry>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl KernelHandExecutor {
|
||||||
|
pub fn new(hands: Arc<HandRegistry>) -> Self {
|
||||||
|
Self { hands }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[async_trait]
|
||||||
|
impl HandExecutor for KernelHandExecutor {
|
||||||
|
async fn execute_hand(
|
||||||
|
&self,
|
||||||
|
hand_id: &str,
|
||||||
|
agent_id: &AgentId,
|
||||||
|
input: Value,
|
||||||
|
) -> Result<Value> {
|
||||||
|
let context = zclaw_hands::HandContext {
|
||||||
|
agent_id: agent_id.clone(),
|
||||||
|
working_dir: None,
|
||||||
|
env: std::collections::HashMap::new(),
|
||||||
|
timeout_secs: 300,
|
||||||
|
callback_url: None,
|
||||||
|
};
|
||||||
|
let result = self.hands.execute(hand_id, &context, input).await?;
|
||||||
|
if result.success {
|
||||||
|
Ok(result.output)
|
||||||
|
} else {
|
||||||
|
Ok(json!({
|
||||||
|
"hand_id": hand_id,
|
||||||
|
"status": "failed",
|
||||||
|
"error": result.error.unwrap_or_else(|| "Unknown hand execution error".to_string()),
|
||||||
|
"output": result.output,
|
||||||
|
"duration_ms": result.duration_ms,
|
||||||
|
}))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -64,6 +64,7 @@ impl Kernel {
|
|||||||
)
|
)
|
||||||
.with_model(&model)
|
.with_model(&model)
|
||||||
.with_skill_executor(self.skill_executor.clone())
|
.with_skill_executor(self.skill_executor.clone())
|
||||||
|
.with_hand_executor(self.hand_executor.clone())
|
||||||
.with_max_tokens(agent_config.max_tokens.unwrap_or_else(|| self.config.max_tokens()))
|
.with_max_tokens(agent_config.max_tokens.unwrap_or_else(|| self.config.max_tokens()))
|
||||||
.with_temperature(agent_config.temperature.unwrap_or_else(|| self.config.temperature()))
|
.with_temperature(agent_config.temperature.unwrap_or_else(|| self.config.temperature()))
|
||||||
.with_compaction_threshold(
|
.with_compaction_threshold(
|
||||||
@@ -176,6 +177,7 @@ impl Kernel {
|
|||||||
)
|
)
|
||||||
.with_model(&model)
|
.with_model(&model)
|
||||||
.with_skill_executor(self.skill_executor.clone())
|
.with_skill_executor(self.skill_executor.clone())
|
||||||
|
.with_hand_executor(self.hand_executor.clone())
|
||||||
.with_max_tokens(agent_config.max_tokens.unwrap_or_else(|| self.config.max_tokens()))
|
.with_max_tokens(agent_config.max_tokens.unwrap_or_else(|| self.config.max_tokens()))
|
||||||
.with_temperature(agent_config.temperature.unwrap_or_else(|| self.config.temperature()))
|
.with_temperature(agent_config.temperature.unwrap_or_else(|| self.config.temperature()))
|
||||||
.with_compaction_threshold(
|
.with_compaction_threshold(
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ use zclaw_skills::SkillRegistry;
|
|||||||
use zclaw_hands::{HandRegistry, hands::{BrowserHand, QuizHand, ResearcherHand, CollectorHand, ClipHand, TwitterHand, ReminderHand, quiz::LlmQuizGenerator}};
|
use zclaw_hands::{HandRegistry, hands::{BrowserHand, QuizHand, ResearcherHand, CollectorHand, ClipHand, TwitterHand, ReminderHand, quiz::LlmQuizGenerator}};
|
||||||
|
|
||||||
pub use adapters::KernelSkillExecutor;
|
pub use adapters::KernelSkillExecutor;
|
||||||
|
pub use adapters::KernelHandExecutor;
|
||||||
pub use messaging::ChatModeConfig;
|
pub use messaging::ChatModeConfig;
|
||||||
|
|
||||||
/// The ZCLAW Kernel
|
/// The ZCLAW Kernel
|
||||||
@@ -40,6 +41,7 @@ pub struct Kernel {
|
|||||||
llm_completer: Arc<dyn zclaw_skills::LlmCompleter>,
|
llm_completer: Arc<dyn zclaw_skills::LlmCompleter>,
|
||||||
skills: Arc<SkillRegistry>,
|
skills: Arc<SkillRegistry>,
|
||||||
skill_executor: Arc<KernelSkillExecutor>,
|
skill_executor: Arc<KernelSkillExecutor>,
|
||||||
|
hand_executor: Arc<KernelHandExecutor>,
|
||||||
hands: Arc<HandRegistry>,
|
hands: Arc<HandRegistry>,
|
||||||
/// Cached hand configs (populated at boot, used for tool registry)
|
/// Cached hand configs (populated at boot, used for tool registry)
|
||||||
hand_configs: Vec<zclaw_hands::HandConfig>,
|
hand_configs: Vec<zclaw_hands::HandConfig>,
|
||||||
@@ -105,6 +107,9 @@ impl Kernel {
|
|||||||
// Create skill executor
|
// Create skill executor
|
||||||
let skill_executor = Arc::new(KernelSkillExecutor::new(skills.clone(), driver.clone()));
|
let skill_executor = Arc::new(KernelSkillExecutor::new(skills.clone(), driver.clone()));
|
||||||
|
|
||||||
|
// Create hand executor — bridges HandTool calls to the HandRegistry
|
||||||
|
let hand_executor = Arc::new(KernelHandExecutor::new(hands.clone()));
|
||||||
|
|
||||||
// Create LLM completer for skill system (shared with skill_executor)
|
// Create LLM completer for skill system (shared with skill_executor)
|
||||||
let llm_completer: Arc<dyn zclaw_skills::LlmCompleter> =
|
let llm_completer: Arc<dyn zclaw_skills::LlmCompleter> =
|
||||||
Arc::new(adapters::LlmDriverAdapter {
|
Arc::new(adapters::LlmDriverAdapter {
|
||||||
@@ -152,6 +157,7 @@ impl Kernel {
|
|||||||
llm_completer,
|
llm_completer,
|
||||||
skills,
|
skills,
|
||||||
skill_executor,
|
skill_executor,
|
||||||
|
hand_executor,
|
||||||
hands,
|
hands,
|
||||||
hand_configs,
|
hand_configs,
|
||||||
trigger_manager,
|
trigger_manager,
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ use zclaw_types::{AgentId, SessionId, Message, Result};
|
|||||||
|
|
||||||
use crate::driver::{LlmDriver, CompletionRequest, ContentBlock};
|
use crate::driver::{LlmDriver, CompletionRequest, ContentBlock};
|
||||||
use crate::stream::StreamChunk;
|
use crate::stream::StreamChunk;
|
||||||
use crate::tool::{ToolRegistry, ToolContext, SkillExecutor};
|
use crate::tool::{ToolRegistry, ToolContext, SkillExecutor, HandExecutor};
|
||||||
use crate::tool::builtin::PathValidator;
|
use crate::tool::builtin::PathValidator;
|
||||||
use crate::growth::GrowthIntegration;
|
use crate::growth::GrowthIntegration;
|
||||||
use crate::compaction::{self, CompactionConfig};
|
use crate::compaction::{self, CompactionConfig};
|
||||||
@@ -28,6 +28,7 @@ pub struct AgentLoop {
|
|||||||
max_tokens: u32,
|
max_tokens: u32,
|
||||||
temperature: f32,
|
temperature: f32,
|
||||||
skill_executor: Option<Arc<dyn SkillExecutor>>,
|
skill_executor: Option<Arc<dyn SkillExecutor>>,
|
||||||
|
hand_executor: Option<Arc<dyn HandExecutor>>,
|
||||||
path_validator: Option<PathValidator>,
|
path_validator: Option<PathValidator>,
|
||||||
/// Growth system integration (optional)
|
/// Growth system integration (optional)
|
||||||
growth: Option<GrowthIntegration>,
|
growth: Option<GrowthIntegration>,
|
||||||
@@ -64,6 +65,7 @@ impl AgentLoop {
|
|||||||
max_tokens: 16384,
|
max_tokens: 16384,
|
||||||
temperature: 0.7,
|
temperature: 0.7,
|
||||||
skill_executor: None,
|
skill_executor: None,
|
||||||
|
hand_executor: None,
|
||||||
path_validator: None,
|
path_validator: None,
|
||||||
growth: None,
|
growth: None,
|
||||||
compaction_threshold: 0,
|
compaction_threshold: 0,
|
||||||
@@ -81,6 +83,12 @@ impl AgentLoop {
|
|||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Set the hand executor for dispatching Hand tool calls to HandRegistry
|
||||||
|
pub fn with_hand_executor(mut self, executor: Arc<dyn HandExecutor>) -> Self {
|
||||||
|
self.hand_executor = Some(executor);
|
||||||
|
self
|
||||||
|
}
|
||||||
|
|
||||||
/// Set the path validator for file system operations
|
/// Set the path validator for file system operations
|
||||||
pub fn with_path_validator(mut self, validator: PathValidator) -> Self {
|
pub fn with_path_validator(mut self, validator: PathValidator) -> Self {
|
||||||
self.path_validator = Some(validator);
|
self.path_validator = Some(validator);
|
||||||
@@ -199,6 +207,7 @@ impl AgentLoop {
|
|||||||
working_directory: working_dir,
|
working_directory: working_dir,
|
||||||
session_id: Some(session_id.to_string()),
|
session_id: Some(session_id.to_string()),
|
||||||
skill_executor: self.skill_executor.clone(),
|
skill_executor: self.skill_executor.clone(),
|
||||||
|
hand_executor: self.hand_executor.clone(),
|
||||||
path_validator: Some(path_validator),
|
path_validator: Some(path_validator),
|
||||||
event_sender: None,
|
event_sender: None,
|
||||||
}
|
}
|
||||||
@@ -567,6 +576,7 @@ impl AgentLoop {
|
|||||||
let tools = self.tools.clone();
|
let tools = self.tools.clone();
|
||||||
let middleware_chain = self.middleware_chain.clone();
|
let middleware_chain = self.middleware_chain.clone();
|
||||||
let skill_executor = self.skill_executor.clone();
|
let skill_executor = self.skill_executor.clone();
|
||||||
|
let hand_executor = self.hand_executor.clone();
|
||||||
let path_validator = self.path_validator.clone();
|
let path_validator = self.path_validator.clone();
|
||||||
let agent_id = self.agent_id.clone();
|
let agent_id = self.agent_id.clone();
|
||||||
let model = self.model.clone();
|
let model = self.model.clone();
|
||||||
@@ -849,6 +859,7 @@ impl AgentLoop {
|
|||||||
working_directory: working_dir,
|
working_directory: working_dir,
|
||||||
session_id: Some(session_id_clone.to_string()),
|
session_id: Some(session_id_clone.to_string()),
|
||||||
skill_executor: skill_executor.clone(),
|
skill_executor: skill_executor.clone(),
|
||||||
|
hand_executor: hand_executor.clone(),
|
||||||
path_validator: Some(pv),
|
path_validator: Some(pv),
|
||||||
event_sender: Some(tx.clone()),
|
event_sender: Some(tx.clone()),
|
||||||
};
|
};
|
||||||
@@ -903,6 +914,7 @@ impl AgentLoop {
|
|||||||
working_directory: working_dir,
|
working_directory: working_dir,
|
||||||
session_id: Some(session_id_clone.to_string()),
|
session_id: Some(session_id_clone.to_string()),
|
||||||
skill_executor: skill_executor.clone(),
|
skill_executor: skill_executor.clone(),
|
||||||
|
hand_executor: hand_executor.clone(),
|
||||||
path_validator: Some(pv),
|
path_validator: Some(pv),
|
||||||
event_sender: Some(tx.clone()),
|
event_sender: Some(tx.clone()),
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -74,12 +74,27 @@ pub struct SkillDetail {
|
|||||||
pub capabilities: Vec<String>,
|
pub capabilities: Vec<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Hand executor trait for runtime hand execution
|
||||||
|
/// This allows tools (HandTool) to execute hands without direct dependency on zclaw-hands
|
||||||
|
#[async_trait]
|
||||||
|
pub trait HandExecutor: Send + Sync {
|
||||||
|
/// Execute a hand by ID, returning the output as JSON
|
||||||
|
async fn execute_hand(
|
||||||
|
&self,
|
||||||
|
hand_id: &str,
|
||||||
|
agent_id: &AgentId,
|
||||||
|
input: Value,
|
||||||
|
) -> Result<Value>;
|
||||||
|
}
|
||||||
|
|
||||||
/// Context provided to tool execution
|
/// Context provided to tool execution
|
||||||
pub struct ToolContext {
|
pub struct ToolContext {
|
||||||
pub agent_id: AgentId,
|
pub agent_id: AgentId,
|
||||||
pub working_directory: Option<String>,
|
pub working_directory: Option<String>,
|
||||||
pub session_id: Option<String>,
|
pub session_id: Option<String>,
|
||||||
pub skill_executor: Option<Arc<dyn SkillExecutor>>,
|
pub skill_executor: Option<Arc<dyn SkillExecutor>>,
|
||||||
|
/// Hand executor for dispatching Hand tool calls to the HandRegistry
|
||||||
|
pub hand_executor: Option<Arc<dyn HandExecutor>>,
|
||||||
/// Path validator for file system operations
|
/// Path validator for file system operations
|
||||||
pub path_validator: Option<PathValidator>,
|
pub path_validator: Option<PathValidator>,
|
||||||
/// Optional event sender for streaming tool progress to the frontend.
|
/// Optional event sender for streaming tool progress to the frontend.
|
||||||
@@ -94,6 +109,7 @@ impl std::fmt::Debug for ToolContext {
|
|||||||
.field("working_directory", &self.working_directory)
|
.field("working_directory", &self.working_directory)
|
||||||
.field("session_id", &self.session_id)
|
.field("session_id", &self.session_id)
|
||||||
.field("skill_executor", &self.skill_executor.as_ref().map(|_| "SkillExecutor"))
|
.field("skill_executor", &self.skill_executor.as_ref().map(|_| "SkillExecutor"))
|
||||||
|
.field("hand_executor", &self.hand_executor.as_ref().map(|_| "HandExecutor"))
|
||||||
.field("path_validator", &self.path_validator.as_ref().map(|_| "PathValidator"))
|
.field("path_validator", &self.path_validator.as_ref().map(|_| "PathValidator"))
|
||||||
.field("event_sender", &self.event_sender.as_ref().map(|_| "Sender<LoopEvent>"))
|
.field("event_sender", &self.event_sender.as_ref().map(|_| "Sender<LoopEvent>"))
|
||||||
.finish()
|
.finish()
|
||||||
@@ -107,6 +123,7 @@ impl Clone for ToolContext {
|
|||||||
working_directory: self.working_directory.clone(),
|
working_directory: self.working_directory.clone(),
|
||||||
session_id: self.session_id.clone(),
|
session_id: self.session_id.clone(),
|
||||||
skill_executor: self.skill_executor.clone(),
|
skill_executor: self.skill_executor.clone(),
|
||||||
|
hand_executor: self.hand_executor.clone(),
|
||||||
path_validator: self.path_validator.clone(),
|
path_validator: self.path_validator.clone(),
|
||||||
event_sender: self.event_sender.clone(),
|
event_sender: self.event_sender.clone(),
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -139,6 +139,7 @@ mod tests {
|
|||||||
working_directory: None,
|
working_directory: None,
|
||||||
session_id: None,
|
session_id: None,
|
||||||
skill_executor: None,
|
skill_executor: None,
|
||||||
|
hand_executor: None,
|
||||||
path_validator,
|
path_validator,
|
||||||
event_sender: None,
|
event_sender: None,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -162,6 +162,7 @@ mod tests {
|
|||||||
working_directory: None,
|
working_directory: None,
|
||||||
session_id: None,
|
session_id: None,
|
||||||
skill_executor: None,
|
skill_executor: None,
|
||||||
|
hand_executor: None,
|
||||||
path_validator,
|
path_validator,
|
||||||
event_sender: None,
|
event_sender: None,
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -78,21 +78,26 @@ impl Tool for HandTool {
|
|||||||
self.input_schema.clone()
|
self.input_schema.clone()
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn execute(&self, input: Value, _context: &ToolContext) -> Result<Value> {
|
async fn execute(&self, input: Value, context: &ToolContext) -> Result<Value> {
|
||||||
// Hand execution is delegated to HandRegistry via the kernel's
|
// Delegate to the HandExecutor (bridged from HandRegistry via kernel).
|
||||||
// hand execution path. This tool acts as the LLM-facing interface.
|
// If no hand_executor is available (e.g., standalone runtime without kernel),
|
||||||
// The actual execution is handled by the HandRegistry when the
|
// return a descriptive error so the LLM knows the hand is unavailable.
|
||||||
// kernel processes the tool call.
|
match &context.hand_executor {
|
||||||
|
Some(executor) => {
|
||||||
// For now, return a structured result that indicates the hand was invoked.
|
executor.execute_hand(&self.hand_id, &context.agent_id, input).await
|
||||||
// The kernel's hand execution layer will handle the actual execution
|
}
|
||||||
// and emit HandStart/HandEnd events.
|
None => {
|
||||||
Ok(json!({
|
Ok(json!({
|
||||||
"hand_id": self.hand_id,
|
"hand_id": self.hand_id,
|
||||||
"status": "invoked",
|
"status": "unavailable",
|
||||||
"input": input,
|
"error": format!(
|
||||||
"message": format!("Hand '{}' invoked successfully", self.hand_id)
|
"Hand '{}' cannot execute: no hand executor configured. \
|
||||||
}))
|
This usually means the kernel is not running or hands are not registered.",
|
||||||
|
self.hand_id
|
||||||
|
)
|
||||||
|
}))
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -130,13 +135,14 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_hand_tool_execute() {
|
async fn test_hand_tool_execute_no_executor() {
|
||||||
let tool = HandTool::from_config("quiz", "Generate quizzes", None);
|
let tool = HandTool::from_config("quiz", "Generate quizzes", None);
|
||||||
let ctx = ToolContext {
|
let ctx = ToolContext {
|
||||||
agent_id: zclaw_types::AgentId::new(),
|
agent_id: zclaw_types::AgentId::new(),
|
||||||
working_directory: None,
|
working_directory: None,
|
||||||
session_id: None,
|
session_id: None,
|
||||||
skill_executor: None,
|
skill_executor: None,
|
||||||
|
hand_executor: None,
|
||||||
path_validator: None,
|
path_validator: None,
|
||||||
event_sender: None,
|
event_sender: None,
|
||||||
};
|
};
|
||||||
@@ -144,6 +150,6 @@ mod tests {
|
|||||||
assert!(result.is_ok());
|
assert!(result.is_ok());
|
||||||
let val = result.unwrap();
|
let val = result.unwrap();
|
||||||
assert_eq!(val["hand_id"], "quiz");
|
assert_eq!(val["hand_id"], "quiz");
|
||||||
assert_eq!(val["status"], "invoked");
|
assert_eq!(val["status"], "unavailable");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user