mirror of
https://github.com/RightNow-AI/openfang.git
synced 2026-07-30 06:32:17 +00:00
bug fixes
This commit is contained in:
@@ -73,6 +73,10 @@ impl ChannelBridgeHandle for KernelBridgeAdapter {
|
||||
.send_message(agent_id, message)
|
||||
.await
|
||||
.map_err(|e| format!("{e}"))?;
|
||||
// Silent/NO_REPLY responses should not be forwarded to channels
|
||||
if result.silent {
|
||||
return Ok(String::new());
|
||||
}
|
||||
Ok(result.response)
|
||||
}
|
||||
|
||||
|
||||
@@ -374,8 +374,11 @@ pub async fn send_message(
|
||||
// Strip <think>...</think> blocks from model output
|
||||
let cleaned = crate::ws::strip_think_tags(&result.response);
|
||||
|
||||
// Guard: ensure we never return an empty response to the client
|
||||
let response = if cleaned.trim().is_empty() {
|
||||
// If the agent intentionally returned a silent/NO_REPLY response,
|
||||
// return an empty string — don't generate debug fallback text.
|
||||
let response = if result.silent {
|
||||
String::new()
|
||||
} else if cleaned.trim().is_empty() {
|
||||
format!(
|
||||
"[The agent completed processing but returned no text response. ({} in / {} out | {} iter)]",
|
||||
result.total_usage.input_tokens,
|
||||
@@ -3431,12 +3434,14 @@ pub async fn clawhub_search(
|
||||
let cache_dir = state.kernel.config.home_dir.join(".cache").join("clawhub");
|
||||
let client = openfang_skills::clawhub::ClawHubClient::new(cache_dir);
|
||||
|
||||
let skills_dir = state.kernel.config.home_dir.join("skills");
|
||||
match client.search(&query, limit).await {
|
||||
Ok(results) => {
|
||||
let items: Vec<serde_json::Value> = results
|
||||
.results
|
||||
.iter()
|
||||
.map(|e| {
|
||||
let installed = skills_dir.join(&e.slug).exists();
|
||||
serde_json::json!({
|
||||
"slug": e.slug,
|
||||
"name": e.display_name,
|
||||
@@ -3444,6 +3449,7 @@ pub async fn clawhub_search(
|
||||
"version": e.version,
|
||||
"score": e.score,
|
||||
"updated_at": e.updated_at,
|
||||
"installed": installed,
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
@@ -3508,12 +3514,18 @@ pub async fn clawhub_browse(
|
||||
let cache_dir = state.kernel.config.home_dir.join(".cache").join("clawhub");
|
||||
let client = openfang_skills::clawhub::ClawHubClient::new(cache_dir);
|
||||
|
||||
let skills_dir = state.kernel.config.home_dir.join("skills");
|
||||
match client.browse(sort, limit, cursor).await {
|
||||
Ok(results) => {
|
||||
let items: Vec<serde_json::Value> = results
|
||||
.items
|
||||
.iter()
|
||||
.map(clawhub_browse_entry_to_json)
|
||||
.map(|entry| {
|
||||
let mut json = clawhub_browse_entry_to_json(entry);
|
||||
let installed = skills_dir.join(&entry.slug).exists();
|
||||
json["installed"] = serde_json::json!(installed);
|
||||
json
|
||||
})
|
||||
.collect();
|
||||
let resp = serde_json::json!({
|
||||
"items": items,
|
||||
|
||||
@@ -2293,7 +2293,7 @@
|
||||
<div class="flex gap-3 items-center">
|
||||
<span class="text-xs text-dim" x-show="skill.version" x-text="'v' + skill.version"></span>
|
||||
</div>
|
||||
<button class="btn btn-primary btn-sm" @click.stop="installFromClawHub(skill.slug)" :disabled="installingSlug === skill.slug || isSkillInstalled(skill.slug)" x-text="isSkillInstalled(skill.slug) ? 'Installed' : installingSlug === skill.slug ? 'Installing...' : 'Install'"></button>
|
||||
<button class="btn btn-primary btn-sm" @click.stop="installFromClawHub(skill.slug)" :disabled="installingSlug === skill.slug || skill.installed || isSkillInstalled(skill.slug)" x-text="skill.installed || isSkillInstalled(skill.slug) ? 'Installed' : installingSlug === skill.slug ? 'Installing...' : 'Install'"></button>
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
@@ -2316,7 +2316,7 @@
|
||||
<span class="text-xs text-dim" x-show="skill.stars" x-text="skill.stars + ' stars'"></span>
|
||||
<span class="text-xs text-dim" x-show="skill.version" x-text="'v' + skill.version"></span>
|
||||
</div>
|
||||
<button class="btn btn-primary btn-sm" @click.stop="installFromClawHub(skill.slug)" :disabled="installingSlug === skill.slug || isSkillInstalled(skill.slug)" x-text="isSkillInstalled(skill.slug) ? 'Installed' : installingSlug === skill.slug ? 'Installing...' : 'Install'"></button>
|
||||
<button class="btn btn-primary btn-sm" @click.stop="installFromClawHub(skill.slug)" :disabled="installingSlug === skill.slug || skill.installed || isSkillInstalled(skill.slug)" x-text="skill.installed || isSkillInstalled(skill.slug) ? 'Installed' : installingSlug === skill.slug ? 'Installing...' : 'Install'"></button>
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
@@ -35,8 +35,10 @@ impl OpenAIDriver {
|
||||
}
|
||||
|
||||
/// True if this provider is Moonshot/Kimi and requires reasoning_content on assistant messages with tool_calls.
|
||||
fn kimi_needs_reasoning_content(&self, model: &str) -> bool {
|
||||
self.base_url.contains("moonshot") || model.to_lowercase().contains("kimi")
|
||||
fn needs_reasoning_content(&self, model: &str) -> bool {
|
||||
self.base_url.contains("moonshot")
|
||||
|| model.to_lowercase().contains("kimi")
|
||||
|| model.to_lowercase().contains("reasoner")
|
||||
}
|
||||
|
||||
/// Create a driver with additional HTTP headers (e.g. for Copilot IDE auth).
|
||||
@@ -295,6 +297,7 @@ impl LlmDriver for OpenAIDriver {
|
||||
(Role::Assistant, MessageContent::Blocks(blocks)) => {
|
||||
let mut text_parts = Vec::new();
|
||||
let mut tool_calls = Vec::new();
|
||||
let mut reasoning_text = String::new();
|
||||
for block in blocks {
|
||||
match block {
|
||||
ContentBlock::Text { text, .. } => text_parts.push(text.clone()),
|
||||
@@ -308,17 +311,16 @@ impl LlmDriver for OpenAIDriver {
|
||||
},
|
||||
});
|
||||
}
|
||||
ContentBlock::Thinking { .. } => {}
|
||||
ContentBlock::Thinking { thinking, .. } => {
|
||||
reasoning_text = thinking.clone();
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
let has_tool_calls = !tool_calls.is_empty();
|
||||
let needs_reasoning = self.needs_reasoning_content(&request.model);
|
||||
oai_messages.push(OaiMessage {
|
||||
role: "assistant".to_string(),
|
||||
// ZHIPU (GLM) rejects assistant messages where content is
|
||||
// null or omitted when tool_calls are present (error 1214).
|
||||
// Always send an empty string so every OpenAI-compat
|
||||
// provider gets a valid payload.
|
||||
content: if text_parts.is_empty() {
|
||||
if has_tool_calls {
|
||||
Some(OaiMessageContent::Text(String::new()))
|
||||
@@ -334,8 +336,8 @@ impl LlmDriver for OpenAIDriver {
|
||||
Some(tool_calls)
|
||||
},
|
||||
tool_call_id: None,
|
||||
reasoning_content: if has_tool_calls && self.kimi_needs_reasoning_content(&request.model) {
|
||||
Some(String::new())
|
||||
reasoning_content: if needs_reasoning {
|
||||
Some(if reasoning_text.is_empty() { String::new() } else { reasoning_text })
|
||||
} else {
|
||||
None
|
||||
},
|
||||
@@ -377,7 +379,7 @@ impl LlmDriver for OpenAIDriver {
|
||||
messages: oai_messages,
|
||||
max_tokens: mt,
|
||||
max_completion_tokens: mct,
|
||||
temperature: if self.kimi_needs_reasoning_content(&request.model) {
|
||||
temperature: if self.needs_reasoning_content(&request.model) {
|
||||
// Kimi with thinking disabled uses fixed 0.6 for multi-turn compatibility.
|
||||
Some(0.6)
|
||||
} else if temperature_must_be_one(&request.model) {
|
||||
@@ -391,7 +393,7 @@ impl LlmDriver for OpenAIDriver {
|
||||
tool_choice,
|
||||
stream: false,
|
||||
stream_options: None,
|
||||
thinking: if self.kimi_needs_reasoning_content(&request.model) {
|
||||
thinking: if self.needs_reasoning_content(&request.model) {
|
||||
Some(serde_json::json!({"type": "disabled"}))
|
||||
} else {
|
||||
None
|
||||
@@ -718,6 +720,7 @@ impl LlmDriver for OpenAIDriver {
|
||||
(Role::Assistant, MessageContent::Blocks(blocks)) => {
|
||||
let mut text_parts = Vec::new();
|
||||
let mut tool_calls_out = Vec::new();
|
||||
let mut reasoning_text = String::new();
|
||||
for block in blocks {
|
||||
match block {
|
||||
ContentBlock::Text { text, .. } => text_parts.push(text.clone()),
|
||||
@@ -731,11 +734,14 @@ impl LlmDriver for OpenAIDriver {
|
||||
},
|
||||
});
|
||||
}
|
||||
ContentBlock::Thinking { .. } => {}
|
||||
ContentBlock::Thinking { thinking, .. } => {
|
||||
reasoning_text = thinking.clone();
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
let has_tool_calls = !tool_calls_out.is_empty();
|
||||
let needs_reasoning = self.needs_reasoning_content(&request.model);
|
||||
oai_messages.push(OaiMessage {
|
||||
role: "assistant".to_string(),
|
||||
content: if text_parts.is_empty() {
|
||||
@@ -753,8 +759,8 @@ impl LlmDriver for OpenAIDriver {
|
||||
Some(tool_calls_out)
|
||||
},
|
||||
tool_call_id: None,
|
||||
reasoning_content: if has_tool_calls && self.kimi_needs_reasoning_content(&request.model) {
|
||||
Some(String::new())
|
||||
reasoning_content: if needs_reasoning {
|
||||
Some(if reasoning_text.is_empty() { String::new() } else { reasoning_text })
|
||||
} else {
|
||||
None
|
||||
},
|
||||
@@ -796,7 +802,7 @@ impl LlmDriver for OpenAIDriver {
|
||||
messages: oai_messages,
|
||||
max_tokens: mt,
|
||||
max_completion_tokens: mct,
|
||||
temperature: if self.kimi_needs_reasoning_content(&request.model) {
|
||||
temperature: if self.needs_reasoning_content(&request.model) {
|
||||
Some(0.6)
|
||||
} else if temperature_must_be_one(&request.model) {
|
||||
Some(1.0)
|
||||
@@ -809,7 +815,7 @@ impl LlmDriver for OpenAIDriver {
|
||||
tool_choice,
|
||||
stream: true,
|
||||
stream_options: Some(serde_json::json!({"include_usage": true})),
|
||||
thinking: if self.kimi_needs_reasoning_content(&request.model) {
|
||||
thinking: if self.needs_reasoning_content(&request.model) {
|
||||
Some(serde_json::json!({"type": "disabled"}))
|
||||
} else {
|
||||
None
|
||||
|
||||
@@ -557,6 +557,10 @@ pub fn is_mcp_tool(name: &str) -> bool {
|
||||
}
|
||||
|
||||
/// Extract server name from an MCP tool name.
|
||||
///
|
||||
/// Falls back to first-underscore heuristic, but prefer
|
||||
/// `extract_mcp_server_from_known()` which handles server names containing
|
||||
/// hyphens (normalized to underscores) correctly.
|
||||
pub fn extract_mcp_server(tool_name: &str) -> Option<&str> {
|
||||
if !tool_name.starts_with("mcp_") {
|
||||
return None;
|
||||
@@ -565,6 +569,30 @@ pub fn extract_mcp_server(tool_name: &str) -> Option<&str> {
|
||||
rest.find('_').map(|pos| &rest[..pos])
|
||||
}
|
||||
|
||||
/// Extract the original server name by matching against known server names.
|
||||
///
|
||||
/// This handles server names with hyphens (e.g. "bocha-search") correctly —
|
||||
/// the normalized prefix "mcp_bocha_search_" is matched against each known
|
||||
/// server's normalized name, returning the original (unhyphenated) name.
|
||||
pub fn extract_mcp_server_from_known<'a>(
|
||||
tool_name: &str,
|
||||
server_names: &[&'a str],
|
||||
) -> Option<&'a str> {
|
||||
if !tool_name.starts_with("mcp_") {
|
||||
return None;
|
||||
}
|
||||
// Sort by length descending so longer (more specific) names match first
|
||||
let mut sorted: Vec<&&str> = server_names.iter().collect();
|
||||
sorted.sort_by_key(|a| std::cmp::Reverse(a.len()));
|
||||
for name in sorted {
|
||||
let prefix = format!("mcp_{}_", normalize_name(name));
|
||||
if tool_name.starts_with(&prefix) {
|
||||
return Some(name);
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Strip the MCP namespace prefix from a tool name.
|
||||
fn strip_mcp_prefix<'a>(server: &str, tool_name: &'a str) -> Option<&'a str> {
|
||||
let prefix = format!("mcp_{}_", normalize_name(server));
|
||||
@@ -627,6 +655,38 @@ mod tests {
|
||||
assert_eq!(extract_mcp_server("file_read"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_mcp_server_from_known_with_hyphens() {
|
||||
// Server "bocha-search" normalized to "bocha_search" in tool prefix
|
||||
let servers = vec!["bocha-search", "github"];
|
||||
let tool = "mcp_bocha_search_bocha_web_search";
|
||||
assert_eq!(
|
||||
extract_mcp_server_from_known(tool, &servers),
|
||||
Some("bocha-search")
|
||||
);
|
||||
// Simple server name still works
|
||||
assert_eq!(
|
||||
extract_mcp_server_from_known("mcp_github_create_issue", &servers),
|
||||
Some("github")
|
||||
);
|
||||
// Non-MCP tool returns None
|
||||
assert_eq!(extract_mcp_server_from_known("file_read", &servers), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_mcp_server_from_known_longest_match() {
|
||||
// "my-api" and "my-api-v2" — should match the longer one
|
||||
let servers = vec!["my-api", "my-api-v2"];
|
||||
assert_eq!(
|
||||
extract_mcp_server_from_known("mcp_my_api_v2_get_users", &servers),
|
||||
Some("my-api-v2")
|
||||
);
|
||||
assert_eq!(
|
||||
extract_mcp_server_from_known("mcp_my_api_list_items", &servers),
|
||||
Some("my-api")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_jsonrpc_initialize() {
|
||||
// Verify the initialize request structure
|
||||
|
||||
@@ -441,8 +441,10 @@ pub async fn execute_tool(
|
||||
// Fallback 1: MCP tools (mcp_{server}_{tool} prefix)
|
||||
if mcp::is_mcp_tool(other) {
|
||||
if let Some(mcp_conns) = mcp_connections {
|
||||
if let Some(server_name) = mcp::extract_mcp_server(other) {
|
||||
let mut conns = mcp_conns.lock().await;
|
||||
let mut conns = mcp_conns.lock().await;
|
||||
let known_names: Vec<String> = conns.iter().map(|c| c.name().to_string()).collect();
|
||||
let known_refs: Vec<&str> = known_names.iter().map(|s| s.as_str()).collect();
|
||||
if let Some(server_name) = mcp::extract_mcp_server_from_known(other, &known_refs) {
|
||||
if let Some(conn) = conns.iter_mut().find(|c| c.name() == server_name) {
|
||||
debug!(
|
||||
tool = other,
|
||||
|
||||
Reference in New Issue
Block a user