From c92004a488389eff9ae6e10037aee582da6e2a57 Mon Sep 17 00:00:00 2001 From: MaximEdogawa Date: Sat, 18 Apr 2026 01:35:51 +0200 Subject: [PATCH 1/2] feat: enhance MCP tools and environment management - Updated the `fetchMcpTools` function to increase the timeout from 3000ms to 15000ms, improving reliability in fetching tools. - Introduced a new `mcpEnvHelpers.ts` module to manage environment variables, including functions for identifying secret keys and building environment maps from form inputs. - Refactored the `AddServerForm` and `McpServerCard` components to utilize the new environment management functions, enhancing the handling of API keys and other environment variables. - Added passthrough environment handling in the tool engine, allowing for better management of environment variables associated with tools. - Implemented new server routes for handling passthrough environment updates, improving the API's capability to manage tool-specific configurations. --- src-tauri/src/infrastructure/http_server.rs | 215 ++++++++++++++- src-tauri/src/modules/bot/agent.rs | 145 ++++++++-- src-tauri/src/modules/bot/mod.rs | 1 + src-tauri/src/modules/bot/search_followup.rs | 260 ++++++++++++++++++ src-tauri/src/modules/mcp/registry.rs | 44 ++- src-tauri/src/modules/mcp/service.rs | 221 +++++++++++++++ src-tauri/src/modules/mcp/types.rs | 6 + src-tauri/src/modules/ollama/service.rs | 7 + src-tauri/src/modules/skills/service.rs | 152 +++++++++- src-tauri/src/modules/skills/types.rs | 4 + src-tauri/src/modules/tool_engine/service.rs | 237 ++++++++++++++-- src-tauri/src/modules/tool_engine/types.rs | 8 +- src-tauri/src/shared/text.rs | 178 +++++++++++- src/modules/mcp/components/AddServerForm.tsx | 64 ++++- src/modules/mcp/components/McpServerCard.tsx | 132 +++++++-- src/modules/mcp/components/McpToolsPanel.tsx | 22 +- src/modules/mcp/index.ts | 2 +- src/modules/mcp/mcpEnvHelpers.ts | 75 +++++ .../toolengine/components/ToolEnginePanel.tsx | 184 +++++++++++++ src/modules/toolengine/index.ts | 26 ++ tools/brave-search/Dockerfile | 13 + tools/mcp-tools.json | 44 ++- tools/skills/game-informer/SKILL.md | 18 ++ 23 files changed, 1960 insertions(+), 98 deletions(-) create mode 100644 src-tauri/src/modules/bot/search_followup.rs create mode 100644 src/modules/mcp/mcpEnvHelpers.ts create mode 100644 tools/brave-search/Dockerfile create mode 100644 tools/skills/game-informer/SKILL.md diff --git a/src-tauri/src/infrastructure/http_server.rs b/src-tauri/src/infrastructure/http_server.rs index beeecd1..95732da 100644 --- a/src-tauri/src/infrastructure/http_server.rs +++ b/src-tauri/src/infrastructure/http_server.rs @@ -15,6 +15,7 @@ use axum::Router; use chrono::Utc; use serde::{Deserialize, Serialize}; use socket2::{Domain, Socket, Type}; +use std::collections::HashMap; use std::convert::Infallible; use std::io::ErrorKind; use std::time::Duration; @@ -112,6 +113,10 @@ pub async fn start_server(state: AppState) { "/v1/toolengine/private-folder", put(handle_toolengine_private_folder_put), ) + .route( + "/v1/toolengine/passthrough-env", + put(handle_toolengine_passthrough_env_put), + ) .route("/v1/toolengine/custom", get(handle_toolengine_custom_list)) .route("/v1/toolengine/custom", post(handle_toolengine_custom_add)) .route( @@ -502,7 +507,16 @@ async fn handle_mcp_servers_list( })? }; Ok(Json(McpServersResponse { - servers: cfg.servers, + servers: cfg + .servers + .iter() + .map(|(k, v)| { + ( + k.clone(), + mcp_service::redact_mcp_server_entry_for_list_response(v), + ) + }) + .collect(), })) } @@ -528,6 +542,7 @@ fn mcp_stdio_identity_ignores_direct_return( args: a0, env: e0, private_host_path: p0, + catalog_passthrough: t0, .. }, ServerEntry::Stdio { @@ -535,9 +550,10 @@ fn mcp_stdio_identity_ignores_direct_return( args: a1, env: e1, private_host_path: p1, + catalog_passthrough: t1, .. }, - ) => c0 == c1 && a0 == a1 && e0 == e1 && p0 == p1, + ) => c0 == c1 && a0 == a1 && e0 == e1 && p0 == p1 && t0 == t1, _ => false, } } @@ -557,7 +573,7 @@ async fn handle_mcp_server_upsert( )); } - if let crate::modules::mcp::types::ServerEntry::Stdio { ref command, .. } = entry { + if let crate::modules::mcp::types::ServerEntry::Stdio { ref command, .. } = &entry { if command.trim().is_empty() { return Err(( StatusCode::BAD_REQUEST, @@ -568,7 +584,7 @@ async fn handle_mcp_server_upsert( } } - let old_entry = { + let (old_entry, entry) = { let _guard = state.mcp_config_mutex.lock().await; let mut cfg = mcp_service::load_or_init_config(&state.mcp_config_path).map_err(|e| { ( @@ -578,6 +594,7 @@ async fn handle_mcp_server_upsert( })?; let old = cfg.servers.get(&name).cloned(); + let entry = mcp_service::merge_stdio_entry_preserving_redacted_secrets(old.as_ref(), entry); if old.as_ref() == Some(&entry) { return Ok((StatusCode::OK, Json(serde_json::json!({ "ok": true })))); } @@ -591,7 +608,7 @@ async fn handle_mcp_server_upsert( ) })?; - old + (old, entry) }; let try_direct_patch = match (&old_entry, &entry) { @@ -763,6 +780,33 @@ async fn handle_toolengine_catalog( .to_string_lossy() .into_owned() }); + let passthrough_configured: Vec = cfg_snap + .as_ref() + .and_then(|c| { + let k = te_service::server_key(&t.id); + match c.servers.get(&k)? { + crate::modules::mcp::types::ServerEntry::Stdio { + catalog_passthrough, + .. + } => { + let mut names: Vec = t + .passthrough_env + .iter() + .filter(|name| { + catalog_passthrough + .get(*name) + .map(|v| !v.trim().is_empty()) + .unwrap_or(false) + }) + .cloned() + .collect(); + names.sort(); + Some(names) + } + _ => None, + } + }) + .unwrap_or_default(); serde_json::json!({ "id": t.id, "name": t.name, @@ -774,6 +818,8 @@ async fn handle_toolengine_catalog( "private_host_path": private_host_resolved, "ignore_robots_txt": t.ignore_robots_txt, "robots_ignore_allowlist": t.robots_ignore_allowlist, + "passthrough_env": t.passthrough_env, + "passthrough_configured_keys": passthrough_configured, }) }) .collect(); @@ -797,6 +843,13 @@ struct PutToolPrivateFolderBody { path: String, } +#[derive(Deserialize)] +struct PutToolPassthroughEnvBody { + tool_id: String, + #[serde(default)] + env: HashMap, +} + async fn handle_toolengine_install( State(state): State, Json(body): Json, @@ -1016,6 +1069,158 @@ async fn handle_toolengine_private_folder_put( Ok((StatusCode::OK, Json(serde_json::json!({ "ok": true })))) } +async fn handle_toolengine_passthrough_env_put( + State(state): State, + Json(body): Json, +) -> Result<(StatusCode, Json), (StatusCode, Json)> { + let tool_id = body.tool_id.trim().to_string(); + if tool_id.is_empty() { + return Err(( + StatusCode::BAD_REQUEST, + Json(ErrorResponse { + error: "tool_id is required".into(), + }), + )); + } + + let catalog = te_service::load_catalog().await.map_err(|e| { + ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(ErrorResponse { error: e }), + ) + })?; + + let entry = catalog + .tools + .iter() + .find(|t| t.id == tool_id) + .ok_or_else(|| { + ( + StatusCode::NOT_FOUND, + Json(ErrorResponse { + error: format!("unknown tool '{tool_id}'"), + }), + ) + })?; + + if entry.passthrough_env.is_empty() { + return Err(( + StatusCode::BAD_REQUEST, + Json(ErrorResponse { + error: "this catalog tool does not declare passthrough_env".into(), + }), + )); + } + + let allowed: HashMap = entry + .passthrough_env + .iter() + .map(|k| (k.clone(), ())) + .collect(); + for key in body.env.keys() { + if !allowed.contains_key(key) { + return Err(( + StatusCode::BAD_REQUEST, + Json(ErrorResponse { + error: format!("unknown passthrough key '{key}' for tool '{tool_id}'"), + }), + )); + } + } + + let bot_id = state + .connection + .lock() + .await + .as_ref() + .map(|c| c.bot_id.clone()); + + { + let _guard = state.mcp_config_mutex.lock().await; + let mut cfg = mcp_service::load_or_init_config(&state.mcp_config_path).map_err(|e| { + ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(ErrorResponse { error: e }), + ) + })?; + + let key = te_service::server_key(&tool_id); + let Some(server_ent) = cfg.servers.get_mut(&key) else { + return Err(( + StatusCode::NOT_FOUND, + Json(ErrorResponse { + error: format!("tool '{tool_id}' is not installed"), + }), + )); + }; + + match server_ent { + crate::modules::mcp::types::ServerEntry::Stdio { + catalog_passthrough, + .. + } => { + for (k, v) in &body.env { + if v.trim().is_empty() { + catalog_passthrough.remove(k); + } else { + catalog_passthrough.insert(k.clone(), v.clone()); + } + } + } + _ => { + return Err(( + StatusCode::INTERNAL_SERVER_ERROR, + Json(ErrorResponse { + error: "tool server entry is not stdio".into(), + }), + )); + } + } + + let host_paths = mcp_service::filesystem_allowed_paths(&cfg); + te_service::sync_workspace_mounted_tools_for_catalog( + &mut cfg, + &host_paths, + &catalog, + &state.mcp_config_path, + bot_id, + ) + .map_err(|e| { + ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(ErrorResponse { error: e }), + ) + })?; + + mcp_service::save_config(&state.mcp_config_path, &cfg).map_err(|e| { + ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(ErrorResponse { error: e }), + ) + })?; + } + + state + .emit_log( + "toolengine", + &format!("passthrough env updated for {tool_id}"), + ) + .await; + + let bg = state.clone(); + tokio::spawn(async move { + if let Err(e) = mcp_service::rebuild_registry_into_state(&bg).await { + bg.emit_log( + "mcp", + &format!("ERROR: MCP registry rebuild failed after passthrough-env update: {e}"), + ) + .await; + } + }); + + Ok((StatusCode::OK, Json(serde_json::json!({ "ok": true })))) +} + async fn handle_toolengine_uninstall( State(state): State, Json(body): Json, diff --git a/src-tauri/src/modules/bot/agent.rs b/src-tauri/src/modules/bot/agent.rs index cb70b18..5a4af28 100644 --- a/src-tauri/src/modules/bot/agent.rs +++ b/src-tauri/src/modules/bot/agent.rs @@ -1,3 +1,4 @@ +use super::search_followup; use crate::modules::memory::{self, MemoryProvider, SessionCommand}; use crate::modules::ollama::keywords::THINK_ON; use crate::modules::ollama::service::{self as ollama, ChatOptions}; @@ -17,6 +18,13 @@ use std::time::{Duration, Instant}; /// used to drop URLs and paraphrase loosely. const MAX_STEPS: usize = 6; +/// Brave Search web calls are billed; allow at most one `brave_web_search` per user message +/// (across all agent steps). Other Brave tools are unchanged. +const MAX_BRAVE_WEB_SEARCH_PER_USER_MESSAGE: u32 = 1; + +const BRAVE_WEB_SEARCH_LIMIT_MSG: &str = "Pengine policy: at most one `brave_web_search` call per user message (cost control). \ +Use the previous search result, answer without another search, or ask the user to narrow the query."; + /// After tool results (agent step ≥1), cap completion tokens. The model should /// put the user-visible answer in `` (see system prompt); this /// cap bounds wall time if it drafts a long ``. ~1024 fits a @@ -33,18 +41,29 @@ const SUMMARY_SYSTEM_PROMPT: &str = "You synthesize tool results for the user. R 1) Use ONLY the text in the user message's Data section (tool outputs). Do not add facts, legal claims, or country-specific rules that are not clearly supported there.\n\ 2) If the Data is insufficient, say so briefly and list what is missing — do not invent answers.\n\ 3) Language: match the user's question where possible.\n\ -4) After the substantive answer, add a final section **Quellen** with a bullet list of every relevant full URL:\n\ +4) After the substantive answer, add a final section **Quellen** with a bullet list of every relevant full URL you relied on:\n\ + - Include URLs from `brave_web_search` results and from every `fetch` block (including lines like `--- fetch (auto: https://…) ---`).\n\ - Copy URLs exactly as they appear in the Data (fetch bodies, HTML links, or Location lines).\n\ - If the Data shows only page text without URLs, write one bullet per tool block naming the fetch target if it appears in the `--- fetch ---` headers or quoted links in the excerpt.\n\ - - Never omit **Quellen** when the Data came from web fetches.\n\ -5) Keep the body concise but do not drop **Quellen** to save space."; - -fn chat_options_for_agent_step(post_tool: bool, user_wants_think: bool) -> ChatOptions { + - Never omit **Quellen** when the Data came from web search or fetches.\n\ +5) Keep the body concise but do not drop **Quellen** to save space.\n\ +6) No chain-of-thought, planning, or English meta: write only text that should appear in the user's chat bubble."; + +/// When the MCP catalog is empty and the user did not enable `/think`, constrain the model to JSON +/// `{\"reply\":...}` so the host can take a single user-visible field (same schema as the summarize pass). +fn chat_options_for_agent_step( + post_tool: bool, + user_wants_think: bool, + json_only_user_reply: bool, +) -> ChatOptions { + let format = (json_only_user_reply && !user_wants_think) + .then_some(ollama::summarize_reply_json_schema()); if !post_tool { ChatOptions { think: Some(user_wants_think), num_predict: None, temperature: None, + format, ..ChatOptions::default() } } else { @@ -52,6 +71,7 @@ fn chat_options_for_agent_step(post_tool: bool, user_wants_think: bool) -> ChatO think: Some(false), num_predict: Some(POST_TOOL_NUM_PREDICT), temperature: Some(POST_TOOL_TEMPERATURE), + format, ..ChatOptions::default() } } @@ -59,10 +79,56 @@ fn chat_options_for_agent_step(post_tool: bool, user_wants_think: bool) -> ChatO /// Cap on tool output fed back to the model. Raw fetch bodies can be 5–10 kB /// of HTML; the model only needs the first screen to answer, and larger -/// feedback balloons the step-1 prompt. Direct replies (answers routed -/// straight to the user) are NOT truncated. +/// feedback balloons the step-1 prompt. Direct replies (non-fetch tools) are +/// not truncated before sending to the user. const TOOL_OUTPUT_CHAR_CAP: usize = 4000; +fn tool_name_is_fetch(name: &str) -> bool { + name.eq_ignore_ascii_case("fetch") + || name + .rsplit_once('.') + .is_some_and(|(_, tail)| tail.eq_ignore_ascii_case("fetch")) +} + +/// After `brave_web_search`, prefetch this many distinct result URLs (one search per message; extra bandwidth here is `fetch` only). +const AUTO_FETCH_TOP_URLS: usize = search_followup::DEFAULT_AUTO_FETCH_CAP; + +async fn append_host_prefetch_after_brave_search( + state: &AppState, + messages: &mut serde_json::Value, + tool_results: &mut Vec<(String, String)>, + search_blob: &str, +) { + let urls = search_followup::extract_fetchable_urls(search_blob, AUTO_FETCH_TOP_URLS); + for url in urls { + state + .emit_log("tool", &format!("[host] auto-fetch {url}")) + .await; + let prep = { + let reg = state.mcp.read().await; + reg.prepare_tool_invocation("fetch", json!({ "url": url.clone() })) + }; + let Ok((provider, tool_name, _, args)) = prep else { + continue; + }; + let text = match provider.call_tool(&tool_name, args).await { + Ok(t) => t, + Err(e) => format!("ERROR: {e}"), + }; + let compacted = compact_tool_output(&text); + let for_model = truncate_for_model(&compacted, TOOL_OUTPUT_CHAR_CAP); + let block_name = format!("fetch (auto: {url})"); + if let Some(arr) = messages.as_array_mut() { + arr.push(json!({ + "role": "tool", + "name": "fetch", + "content": &for_model, + })); + } + tool_results.push((block_name, for_model)); + } +} + fn push_ephemeral_post_tool_reminder(messages: &mut serde_json::Value) { if let Some(arr) = messages.as_array_mut() { arr.push(json!({ @@ -489,7 +555,10 @@ async fn build_system_prompt(state: &AppState, has_tools: bool, has_memory: bool format!( "{PENGINE_OUTPUT_CONTRACT_LEAD}Assistant with tools. Call a tool only for external data; otherwise answer directly. \ - After tool results, answer immediately. Be concise.{fs_hint}{mem_hint}{skills_hint}" + After tool results, answer immediately. Be concise. \ + `brave_web_search` is only in the tool list when the user asked to search the open web (e.g. “search the internet”, “suche im Internet”, “suche nach …”) or a skill’s `requires` matches this turn — otherwise prefer **`fetch`** on any `http(s)` URL you have (including from the user). \ + At most one `brave_web_search` per user message when it is available. \ + After an allowed search, the host may auto-`fetch` several top result URLs — use those excerpts and end with **Quellen** listing every source URL.{fs_hint}{mem_hint}{skills_hint}" ) } @@ -521,6 +590,9 @@ async fn run_model_turn( .as_ref() .is_some_and(|s| !s.diary_only); + let allow_brave_web_search = + skills::allow_brave_web_search_for_message(&state.store_path, user_message); + let mut tool_ctx = { let reg = state.mcp.read().await; reg.select_tools_for_turn( @@ -528,13 +600,14 @@ async fn run_model_turn( &recent_tools, memory_server_key.as_deref(), chat_session_recording, + allow_brave_web_search, ) }; state .emit_log( "tool_ctx", &format!( - "select_ms={} active={}/{} subset={} routing={} recording={} high_risk={} recent_n={}", + "select_ms={} active={}/{} subset={} routing={} recording={} high_risk={} recent_n={} brave_web={}", tool_ctx.select_ms, tool_ctx.active_count, tool_ctx.total_count, @@ -542,7 +615,8 @@ async fn run_model_turn( tool_ctx.routing, chat_session_recording, tool_ctx.high_risk_active, - recent_tools.len() + recent_tools.len(), + allow_brave_web_search ), ) .await; @@ -562,6 +636,7 @@ async fn run_model_turn( let mut tools_supported = true; let empty_tools = json!([]); let mut routing_escalated = false; + let mut brave_web_search_calls_this_message: u32 = 0; // Counts actual tool-result rounds, not loop iterations. A routing escalation // re-enters step 0 with a fresh catalog, so it must not be treated as a // post-tool continuation (no reminder, keep user's think/num_predict). @@ -575,7 +650,8 @@ async fn run_model_turn( &empty_tools }; let post_tool = tool_rounds > 0; - let chat_opts = chat_options_for_agent_step(post_tool, think); + let json_only_user_reply = !has_tools; + let chat_opts = chat_options_for_agent_step(post_tool, think, json_only_user_reply); let inject_post_tool = post_tool; if inject_post_tool { @@ -624,7 +700,7 @@ async fn run_model_turn( routing_escalated = true; tool_ctx = { let reg = state.mcp.read().await; - reg.full_tool_context() + reg.full_tool_context(allow_brave_web_search) }; state .emit_log( @@ -654,7 +730,7 @@ async fn run_model_turn( .await; // Resolve under one lock, then execute in parallel. - let prepared: Vec<_> = { + let mut prepared = { let reg = state.mcp.read().await; tool_calls .iter() @@ -669,9 +745,23 @@ async fn run_model_turn( let resolved = reg.prepare_tool_invocation(&name, args); (name, resolved) }) - .collect() + .collect::>() }; + for (name, res) in &mut prepared { + if !name.eq_ignore_ascii_case("brave_web_search") { + continue; + } + if res.is_err() { + continue; + } + if brave_web_search_calls_this_message >= MAX_BRAVE_WEB_SEARCH_PER_USER_MESSAGE { + *res = Err(BRAVE_WEB_SEARCH_LIMIT_MSG.to_string()); + } else { + brave_web_search_calls_this_message += 1; + } + } + let invoked_names: Vec = prepared.iter().map(|(n, _)| n.clone()).collect(); state.note_tools_used(&invoked_names).await; @@ -684,13 +774,15 @@ async fn run_model_turn( let (p, tn, a) = (provider.clone(), tool_name.clone(), args.clone()); handles.push(tokio::spawn(async move { p.call_tool(&tn, a).await })); } - Err(_) => { - handles.push(tokio::spawn(async { Err("resolve failed".to_string()) })); + Err(e) => { + let e = e.clone(); + handles.push(tokio::spawn(async move { Err(e) })); } } } let mut direct_replies: Vec = Vec::new(); + let mut last_brave_search_blob: Option = None; for (i, handle) in handles.into_iter().enumerate() { let (name, resolved) = &prepared[i]; let (text, is_direct) = match handle.await { @@ -713,11 +805,18 @@ async fn run_model_turn( } }; - if is_direct { + // Fetch output is often XML/HTML snippets; never bypass the model for the user bubble + // (even if `mcp.json` still has `direct_return: true` from an older catalog default). + if is_direct && !tool_name_is_fetch(name) { direct_replies.push(text.clone()); } let compacted = compact_tool_output(&text); let for_model = truncate_for_model(&compacted, TOOL_OUTPUT_CHAR_CAP); + if name.eq_ignore_ascii_case("brave_web_search") + && !for_model.trim_start().starts_with("ERROR:") + { + last_brave_search_blob = Some(for_model.clone()); + } if let Some(arr) = messages.as_array_mut() { arr.push(json!({ "role": "tool", "name": name, "content": &for_model })); } @@ -731,6 +830,11 @@ async fn run_model_turn( .await; tool_rounds += 1; + if let Some(blob) = last_brave_search_blob { + append_host_prefetch_after_brave_search(state, &mut messages, &mut tool_results, &blob) + .await; + } + if !direct_replies.is_empty() { return Ok(TurnResult { text: direct_replies.join("\n\n"), @@ -840,4 +944,11 @@ mod tests { assert!(!ThinkSource::SlashOff.enabled()); assert!(!ThinkSource::Default.enabled()); } + + #[test] + fn fetch_tool_name_detection() { + assert!(tool_name_is_fetch("fetch")); + assert!(tool_name_is_fetch("te_pengine-fetch.fetch")); + assert!(!tool_name_is_fetch("roll_dice")); + } } diff --git a/src-tauri/src/modules/bot/mod.rs b/src-tauri/src/modules/bot/mod.rs index 5001b1c..e42a909 100644 --- a/src-tauri/src/modules/bot/mod.rs +++ b/src-tauri/src/modules/bot/mod.rs @@ -1,4 +1,5 @@ pub mod agent; pub mod commands; pub mod repository; +pub mod search_followup; pub mod service; diff --git a/src-tauri/src/modules/bot/search_followup.rs b/src-tauri/src/modules/bot/search_followup.rs new file mode 100644 index 0000000..8724914 --- /dev/null +++ b/src-tauri/src/modules/bot/search_followup.rs @@ -0,0 +1,260 @@ +//! Extract HTTP(S) URLs from `brave_web_search` tool text and cap how many the host prefetches. +//! +//! The model often answers from SERP snippets alone; one search per message is kept for cost +//! control, but we still attach `fetch` tool output for the top distinct result URLs so the +//! next model step can reason over page text. + +use serde_json::Value; +use std::collections::HashSet; + +/// Maximum number of distinct URLs the host will `fetch` after a single `brave_web_search`. +pub const DEFAULT_AUTO_FETCH_CAP: usize = 5; + +fn trim_url_trailing_junk(mut s: String) -> String { + while let Some(c) = s.chars().last() { + if matches!(c, ')' | ']' | '>' | '.' | ',' | ';' | '"' | '\'') { + s.pop(); + } else { + break; + } + } + s +} + +fn should_skip_url(url: &str) -> bool { + let u = url.to_lowercase(); + // Brave / tracker noise sometimes appears in raw payloads. + u.contains("cdn.search.brave") + || u.contains("brave.com/static") + || u.starts_with("mailto:") + || u.starts_with("tel:") +} + +/// Hosts we skip for host-prefetch after web search — social / aggregators rarely carry the article body. +const SOCIAL_OR_PORTAL_HOST_MARKERS: &[&str] = &[ + "facebook.com", + "instagram.com", + "twitter.com", + "x.com", + "tiktok.com", + "reddit.com", + "linkedin.com", + "pinterest.com", + "wikipedia.org", +]; + +fn url_host_for_policy(url: &str) -> Option { + let t = url.trim(); + let rest = t + .strip_prefix("https://") + .or_else(|| t.strip_prefix("http://"))?; + let host_end = rest + .find(|c| ['/', '?', '#'].contains(&c)) + .unwrap_or(rest.len()); + let host = rest.get(..host_end)?; + if host.is_empty() { + return None; + } + Some(host.to_lowercase()) +} + +fn host_is_social_or_wikipedia(host: &str) -> bool { + let h = host.to_lowercase(); + SOCIAL_OR_PORTAL_HOST_MARKERS + .iter() + .any(|m| h == *m || h.ends_with(&format!(".{m}")) || h.contains(&format!(".{m}"))) +} + +fn host_should_skip_auto_fetch(url: &str) -> bool { + url_host_for_policy(url) + .map(|h| host_is_social_or_wikipedia(&h)) + .unwrap_or(true) +} + +/// Brave JSON: preserve `web.results[].url` order (not a full-tree walk, which reorders URLs). +fn ordered_brave_result_urls(v: &Value) -> Vec { + let Some(web) = v.get("web") else { + return Vec::new(); + }; + let Some(results) = web.get("results").and_then(|x| x.as_array()) else { + return Vec::new(); + }; + let mut out = Vec::new(); + for r in results { + let Some(s) = r.get("url").and_then(|x| x.as_str()) else { + continue; + }; + let u = trim_url_trailing_junk(s.to_string()); + if looks_like_http_url(&u) && !should_skip_url(&u) && !host_should_skip_auto_fetch(&u) { + out.push(u); + } + } + out +} + +fn registrable_core_host(host: &str) -> String { + let h = host.trim().to_lowercase(); + let parts: Vec<&str> = h.split('.').collect(); + if parts.len() >= 2 { + format!("{}.{}", parts[parts.len() - 2], parts[parts.len() - 1]) + } else { + h + } +} + +/// Prefer URLs on the same site as the first organic hit, then other allowed URLs. +fn prioritize_same_site_first(urls: &[String]) -> Vec { + if urls.is_empty() { + return Vec::new(); + } + let Some(first_h) = urls.first().and_then(|u| url_host_for_policy(u)) else { + return urls.to_vec(); + }; + let core = registrable_core_host(&first_h); + let mut same = Vec::new(); + let mut other = Vec::new(); + for u in urls { + if host_should_skip_auto_fetch(u) { + continue; + } + if let Some(h) = url_host_for_policy(u) { + if registrable_core_host(&h) == core { + same.push(u.clone()); + } else { + other.push(u.clone()); + } + } + } + same.extend(other); + same +} + +fn looks_like_http_url(s: &str) -> bool { + let t = s.trim(); + if !(t.starts_with("http://") || t.starts_with("https://")) || t.len() < 12 { + return false; + } + let Some(rest) = t + .strip_prefix("https://") + .or_else(|| t.strip_prefix("http://")) + else { + return false; + }; + let host_end = rest + .find(|c| ['/', '?', '#'].contains(&c)) + .unwrap_or(rest.len()); + let host = &rest[..host_end]; + !host.is_empty() && (host.contains('.') || host == "localhost") +} + +fn collect_urls_from_json(v: &Value, acc: &mut Vec) { + match v { + Value::Object(map) => { + for (k, val) in map { + if k.eq_ignore_ascii_case("url") || k.eq_ignore_ascii_case("link") { + if let Some(s) = val.as_str() { + if looks_like_http_url(s) && !should_skip_url(s) { + acc.push(trim_url_trailing_junk(s.to_string())); + } + } + } + collect_urls_from_json(val, acc); + } + } + Value::Array(a) => { + for x in a { + collect_urls_from_json(x, acc); + } + } + _ => {} + } +} + +fn collect_urls_regex(text: &str, acc: &mut Vec) { + // Broad but conservative: stop at common delimiters after the path. + let re = regex::Regex::new(r#"https?://[^\s\]`"'<>)\]]+"#).expect("valid regex"); + for m in re.find_iter(text) { + let u = trim_url_trailing_junk(m.as_str().to_string()); + if looks_like_http_url(&u) && !should_skip_url(&u) { + acc.push(u); + } + } +} + +/// Ordered, deduplicated HTTP(S) URLs suitable for follow-up `fetch` calls. +pub fn extract_fetchable_urls(search_output: &str, max: usize) -> Vec { + let trimmed = search_output.trim(); + let json_val = serde_json::from_str::(trimmed).ok(); + + let mut raw: Vec = Vec::new(); + if let Some(ref v) = json_val { + let ordered = ordered_brave_result_urls(v); + if !ordered.is_empty() { + raw.extend(prioritize_same_site_first(&ordered)); + } + } + if raw.is_empty() { + if let Some(ref v) = json_val { + collect_urls_from_json(v, &mut raw); + } + collect_urls_regex(trimmed, &mut raw); + } + + let mut seen = HashSet::new(); + let mut out = Vec::new(); + for u in raw { + let key = u.trim().to_string(); + if key.is_empty() + || !looks_like_http_url(&key) + || should_skip_url(&key) + || host_should_skip_auto_fetch(&key) + || !seen.insert(key.clone()) + { + continue; + } + out.push(key); + if out.len() >= max { + break; + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn extracts_urls_from_brave_style_json() { + let j = r#"{"web":{"results":[ + {"url":"https://www.oesterreich.gv.at/de/a","title":"A"}, + {"url":"https://www.oesterreich.gv.at/de/b","title":"B"} + ]}}"#; + let u = extract_fetchable_urls(j, 10); + assert_eq!(u.len(), 2); + assert!(u[0].contains("oesterreich.gv.at")); + } + + #[test] + fn dedupes_and_caps() { + let j = r#"{"url":"https://example.com/x"} + https://example.com/x + https://other.test/y"#; + let u = extract_fetchable_urls(j, 2); + assert_eq!(u.len(), 2); + assert!(u.iter().any(|s| s.contains("example.com"))); + assert!(u.iter().any(|s| s.contains("other.test"))); + } + + #[test] + fn brave_results_skip_social_and_keep_news_site() { + let j = r#"{"web":{"results":[ + {"url":"https://www.facebook.com/officialgameinformer/"}, + {"url":"https://www.gameinformer.com/news"}, + {"url":"https://en.wikipedia.org/wiki/Game_Informer"} + ]}}"#; + let u = extract_fetchable_urls(j, 5); + assert_eq!(u.len(), 1); + assert!(u[0].contains("gameinformer.com")); + } +} diff --git a/src-tauri/src/modules/mcp/registry.rs b/src-tauri/src/modules/mcp/registry.rs index d384797..eb0224f 100644 --- a/src-tauri/src/modules/mcp/registry.rs +++ b/src-tauri/src/modules/mcp/registry.rs @@ -196,6 +196,7 @@ impl ToolRegistry { recent_tool_names: &[String], memory_server: Option<&str>, chat_session_recording: bool, + allow_brave_web_search: bool, ) -> ToolContextSelection { let t0 = Instant::now(); let all = self.all_tools(); @@ -211,6 +212,7 @@ impl ToolRegistry { tools: selected, routing, } => { + let selected = filter_brave_web_search(selected, allow_brave_web_search); let select_ms = t0.elapsed().as_millis() as u64; let high = selected.iter().filter(|t| t.risk == ToolRisk::High).count(); let active = selected.len(); @@ -225,12 +227,14 @@ impl ToolRegistry { } } ToolRoutePlan::FullCatalog => { + let selected = filter_brave_web_search(all, allow_brave_web_search); let select_ms = t0.elapsed().as_millis() as u64; - let high = all.iter().filter(|t| t.risk == ToolRisk::High).count(); + let high = selected.iter().filter(|t| t.risk == ToolRisk::High).count(); + let active = selected.len(); ToolContextSelection { - tools_json: self.cached_ollama_tools.clone(), + tools_json: build_ollama_tools(&selected), total_count: total, - active_count: total, + active_count: active, used_subset: false, routing: "full", select_ms, @@ -241,14 +245,15 @@ impl ToolRegistry { } /// Full catalog (no subset). Used after routing escalation. - pub fn full_tool_context(&self) -> ToolContextSelection { + pub fn full_tool_context(&self, allow_brave_web_search: bool) -> ToolContextSelection { let all = self.all_tools(); let total = self.cached_tool_names.len(); - let high = all.iter().filter(|t| t.risk == ToolRisk::High).count(); + let selected = filter_brave_web_search(all, allow_brave_web_search); + let high = selected.iter().filter(|t| t.risk == ToolRisk::High).count(); ToolContextSelection { - tools_json: self.cached_ollama_tools.clone(), + tools_json: build_ollama_tools(&selected), total_count: total, - active_count: total, + active_count: selected.len(), used_subset: false, routing: "full_escalation", select_ms: 0, @@ -421,6 +426,14 @@ const ROUTING_STOPWORDS: &[&str] = &[ /// intent words, typos) — a tiny token cost for a big correctness win. const ALWAYS_ON_TOOL_NAMES: &[&str] = &["fetch", "time"]; +fn filter_brave_web_search(mut tools: Vec, allow: bool) -> Vec { + if allow { + return tools; + } + tools.retain(|t| !t.name.eq_ignore_ascii_case("brave_web_search")); + tools +} + #[derive(Debug)] enum ToolRoutePlan { FullCatalog, @@ -614,6 +627,21 @@ fn message_suggests_url_fetch(msg: &str) -> bool { "api", "download", "abruf", + "webseite", + "website", + "offiziell", + "öffentlich", + "oeffentlich", + "link ", + "url ", + "quelle", + "nachlesen", + "abrufen", + "dokumentation", + "spec", + "readme", + "oesterreich.gv", + "bundesrecht", ]; HINTS.iter().any(|h| lower.contains(h)) } @@ -631,7 +659,7 @@ fn score_tool_combined( }; s += recent_tool_score(tool, recent_tool_names); if tool.name.eq_ignore_ascii_case("fetch") && message_suggests_url_fetch(user_message) { - s += 14; + s += 22; } s } diff --git a/src-tauri/src/modules/mcp/service.rs b/src-tauri/src/modules/mcp/service.rs index c78cece..4e9ef33 100644 --- a/src-tauri/src/modules/mcp/service.rs +++ b/src-tauri/src/modules/mcp/service.rs @@ -5,6 +5,7 @@ use super::client::McpClient; use super::native; use super::registry::{Provider, ToolRegistry}; use super::types::{McpConfig, ServerEntry}; +use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::Arc; use tauri::Emitter; @@ -120,6 +121,152 @@ fn default_config_value() -> serde_json::Value { }) } +fn redact_podman_docker_env_argv(args: &[String]) -> Vec { + args.iter() + .map(|a| { + let Some(rest) = a.strip_prefix("--env=") else { + return a.clone(); + }; + let Some((name, val)) = rest.split_once('=') else { + return a.clone(); + }; + if val.is_empty() { + return a.clone(); + } + format!("--env={name}=********") + }) + .collect() +} + +fn command_is_podman_or_docker(command: &str) -> bool { + let cmd_trim = command.trim(); + std::path::Path::new(cmd_trim) + .file_name() + .and_then(|s| s.to_str()) + .map(|b| b == "podman" || b == "docker") + .unwrap_or(cmd_trim == "podman" || cmd_trim == "docker") +} + +/// Removes stored catalog secrets and masks `--env=…` values in `podman|docker run` argv before +/// returning `mcp.json` over HTTP (GET `/v1/mcp/servers`). +pub fn redact_mcp_server_entry_for_list_response(entry: &ServerEntry) -> ServerEntry { + match entry { + ServerEntry::Native { .. } => entry.clone(), + ServerEntry::Stdio { + command, + args, + env, + direct_return, + private_host_path, + .. + } => { + let args = if command_is_podman_or_docker(command) { + redact_podman_docker_env_argv(args) + } else { + args.clone() + }; + ServerEntry::Stdio { + command: command.clone(), + args, + env: env.clone(), + direct_return: *direct_return, + private_host_path: private_host_path.clone(), + catalog_passthrough: HashMap::new(), + } + } + } +} + +const REDACTED_ENV_VALUE_PLACEHOLDER: &str = "********"; + +fn is_redacted_podman_env_arg(token: &str) -> bool { + let Some(rest) = token.strip_prefix("--env=") else { + return false; + }; + let Some((_name, val)) = rest.split_once('=') else { + return false; + }; + val == REDACTED_ENV_VALUE_PLACEHOLDER +} + +/// Restores real `podman|docker run --env=…` argv and `catalog_passthrough` when the client PUTs +/// a stdio entry that came from [`redact_mcp_server_entry_for_list_response`] (dashboard round-trip, +/// e.g. toggling `direct_return`), so secrets are not replaced with `********` on disk. +pub fn merge_stdio_entry_preserving_redacted_secrets( + old_entry: Option<&ServerEntry>, + entry: ServerEntry, +) -> ServerEntry { + let ServerEntry::Stdio { + command, + mut args, + env, + direct_return, + private_host_path, + mut catalog_passthrough, + } = entry + else { + return entry; + }; + + let Some(ServerEntry::Stdio { + args: old_args, + catalog_passthrough: old_cp, + .. + }) = old_entry + else { + return ServerEntry::Stdio { + command, + args, + env, + direct_return, + private_host_path, + catalog_passthrough, + }; + }; + + if !command_is_podman_or_docker(&command) { + return ServerEntry::Stdio { + command, + args, + env, + direct_return, + private_host_path, + catalog_passthrough, + }; + } + + let mut restored_any_env = false; + for new_a in &mut args { + if !is_redacted_podman_env_arg(new_a.as_str()) { + continue; + } + let Some(rest) = new_a.strip_prefix("--env=") else { + continue; + }; + let Some((name, _)) = rest.split_once('=') else { + continue; + }; + let prefix = format!("--env={name}="); + if let Some(old_a) = old_args.iter().find(|a| a.starts_with(&prefix)) { + *new_a = old_a.clone(); + restored_any_env = true; + } + } + + if restored_any_env && catalog_passthrough.is_empty() && !old_cp.is_empty() { + catalog_passthrough = old_cp.clone(); + } + + ServerEntry::Stdio { + command, + args, + env, + direct_return, + private_host_path, + catalog_passthrough, + } +} + pub fn load_or_init_config(path: &Path) -> Result { if path.exists() { return read_config(path); @@ -412,4 +559,78 @@ mod tests { assert_eq!(src, "app_data"); assert_eq!(path, PathBuf::from("/tmp/pengine-fake-app/mcp.json")); } + + #[test] + fn list_response_redacts_podman_env_args_and_catalog_passthrough() { + let entry = ServerEntry::Stdio { + command: "podman".into(), + args: vec![ + "run".into(), + "--rm".into(), + "--env=BRAVE_API_KEY=super-secret".into(), + ], + env: HashMap::new(), + direct_return: false, + private_host_path: None, + catalog_passthrough: HashMap::from([("BRAVE_API_KEY".into(), "super-secret".into())]), + }; + let r = redact_mcp_server_entry_for_list_response(&entry); + let ServerEntry::Stdio { + args, + catalog_passthrough, + .. + } = r + else { + panic!("expected stdio"); + }; + assert!( + args.iter().any(|a| a == "--env=BRAVE_API_KEY=********"), + "args={args:?}" + ); + assert!(catalog_passthrough.is_empty()); + } + + #[test] + fn merge_stdio_restores_redacted_env_argv_and_catalog_passthrough() { + let old = ServerEntry::Stdio { + command: "podman".into(), + args: vec![ + "run".into(), + "--rm".into(), + "--env=BRAVE_API_KEY=real-secret".into(), + ], + env: HashMap::new(), + direct_return: false, + private_host_path: None, + catalog_passthrough: HashMap::from([("BRAVE_API_KEY".into(), "real-secret".into())]), + }; + let new = ServerEntry::Stdio { + command: "podman".into(), + args: vec![ + "run".into(), + "--rm".into(), + "--env=BRAVE_API_KEY=********".into(), + ], + env: HashMap::new(), + direct_return: true, + private_host_path: None, + catalog_passthrough: HashMap::new(), + }; + let merged = merge_stdio_entry_preserving_redacted_secrets(Some(&old), new); + let ServerEntry::Stdio { + args, + catalog_passthrough, + direct_return, + .. + } = merged + else { + panic!("expected stdio"); + }; + assert_eq!(args[2], "--env=BRAVE_API_KEY=real-secret"); + assert_eq!( + catalog_passthrough.get("BRAVE_API_KEY").map(String::as_str), + Some("real-secret") + ); + assert!(direct_return); + } } diff --git a/src-tauri/src/modules/mcp/types.rs b/src-tauri/src/modules/mcp/types.rs index 9345ac8..fed1e6a 100644 --- a/src-tauri/src/modules/mcp/types.rs +++ b/src-tauri/src/modules/mcp/types.rs @@ -27,6 +27,8 @@ pub enum ServerEntry { command: String, #[serde(default)] args: Vec, + /// Host environment passed to the spawned `command` (e.g. `npx`). Tool Engine catalog + /// servers normally leave this empty and inject container env via `args` (`podman run --env=…`). #[serde(default)] env: HashMap, /// When true, tool results are returned directly to the user without @@ -37,6 +39,10 @@ pub enum ServerEntry { /// into the container. Defaults to `$APP_DATA/tool-data//`; user overrides land here. #[serde(default, skip_serializing_if = "Option::is_none")] private_host_path: Option, + /// Persisted values for catalog `passthrough_env` keys (injected into `args` as + /// `podman|docker run --env=…`). Not applied to the host `podman` process via `env`. + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + catalog_passthrough: HashMap, }, } diff --git a/src-tauri/src/modules/ollama/service.rs b/src-tauri/src/modules/ollama/service.rs index 404c874..09b782e 100644 --- a/src-tauri/src/modules/ollama/service.rs +++ b/src-tauri/src/modules/ollama/service.rs @@ -285,6 +285,13 @@ fn extract_message( .cloned() .ok_or_else(|| format!("ollama protocol error: missing `message` in response: {body}"))?; + // Ollama thinking-capable models can return a separate `message.thinking` trace + // (see https://docs.ollama.com/capabilities/thinking). Never persist or forward it: + // only `content` is user-visible after normalization. + if let Some(obj) = msg.as_object_mut() { + obj.remove("thinking"); + } + // Strip template-injected reasoning, then apply our reply contract (JSON or // ``) so Telegram and the next-step history never carry plan text. if let Some(content) = msg.get("content").and_then(|v| v.as_str()) { diff --git a/src-tauri/src/modules/skills/service.rs b/src-tauri/src/modules/skills/service.rs index a8b66af..d55592d 100644 --- a/src-tauri/src/modules/skills/service.rs +++ b/src-tauri/src/modules/skills/service.rs @@ -111,7 +111,9 @@ pub const MAX_TOTAL_SKILL_HINT_BYTES: usize = SKILL_HINT_BODY_CAP * 8; const SKILL_HINT_INTRO: &str = "\n\nSkills: follow each recipe exactly — \ it lists WHICH URL and HOW MANY calls. Stop when you can answer; \ -don't probe alternate hosts. The fetch tool is available."; +don't probe alternate hosts. Prefer **`fetch`** whenever you have a concrete URL; use **`brave_web_search`** only when the recipe lists it in `requires` (and this turn matches that skill) or the user explicitly asked to search the open web.\n\ +Portal- or government-specific skills you install yourself apply **only** when the user is clearly asking about that jurisdiction’s government, law, official forms, or public administration — \ +not for recipes, hobbies, general knowledge, software, or unrelated chit-chat. If the topic does not match the skill’s scope, ignore that recipe entirely."; /// Build a system-prompt fragment describing the enabled skills so the agent /// knows when/how to invoke fetch tools for each. Returns `""` if there are @@ -268,6 +270,7 @@ pub fn parse_skill(slug: &str, raw: &str, origin: SkillOrigin) -> Result Result bool { + let u = user_message.to_lowercase(); + const PHRASES: &[&str] = &[ + "search the internet", + "search the web", + "suche im internet", + "suche im internt", + "suche mir im internet", + "such mir im internet", + "such mir im web", + "such mal im internet", + "im internet suchen", + "im web suchen", + "finde mir im internet", + "suche im internet", + "seachr", + "web search", + "internetrecherche", + "recherche im internet", + "online recherchieren", + "google mal", + "duckduckgo", + ]; + if PHRASES.iter().any(|p| u.contains(p)) { + return true; + } + // "suche nach" alone matches too many German sentences; require an explicit web intent nearby. + if u.contains("suche nach") + && (u.contains("internet") + || u.contains("online") + || u.contains("im web") + || u.contains("bei google") + || u.contains("duckduckgo")) + { + return true; + } + false +} + +/// Tags this generic must not alone enable billed web search (e.g. "news" ⊆ "gameinformer news"). +const BRAVE_TAG_DENYLIST: &[&str] = &[ + "news", "info", "help", "guide", "tips", "blog", "home", "page", "data", "list", "links", + "link", "tool", "tools", "apps", "app", "media", "site", "sites", "world", "daily", "live", +]; + +fn skill_triggers_brave_web_search(skill: &Skill, user_message: &str) -> bool { + if !skill + .requires + .iter() + .any(|r| r.eq_ignore_ascii_case("brave_web_search")) + { + return false; + } + let u = user_message.to_lowercase(); + for sub in &skill.brave_allow_substrings { + if sub.len() >= 3 && u.contains(&sub.to_lowercase()) { + return true; + } + } + for t in &skill.tags { + if t.len() < 6 { + continue; + } + if BRAVE_TAG_DENYLIST.iter().any(|g| g.eq_ignore_ascii_case(t)) { + continue; + } + if u.contains(&t.to_lowercase()) { + return true; + } + } + false +} + +/// Expose the billed `brave_web_search` tool only when a skill lists it in `requires` and the +/// user message matches that skill’s `brave_allow_substrings` / tags, or when the user uses an +/// explicit “search the internet”-style phrase. +pub fn allow_brave_web_search_for_message(store_path: &Path, user_message: &str) -> bool { + if user_explicitly_requests_web_search(user_message) { + return true; + } + list_skills(store_path) + .into_iter() + .filter(|s| s.enabled) + .any(|s| skill_triggers_brave_web_search(&s, user_message)) +} + fn split_frontmatter(raw: &str) -> Option<(&str, &str)> { let trimmed = raw.trim_start_matches('\u{feff}'); let rest = trimmed.strip_prefix("---")?; @@ -865,4 +955,64 @@ mod tests { "expected answer-style reminder in:\n{hint}" ); } + + #[test] + fn allow_brave_from_explicit_web_search_phrase() { + let tmp = tempdir().unwrap(); + let fake_store = tmp.path().join("connection.json"); + assert!(allow_brave_web_search_for_message( + &fake_store, + "bitte suche im Internet nach X" + )); + assert!(allow_brave_web_search_for_message( + &fake_store, + "search the internet for penguins" + )); + assert!(allow_brave_web_search_for_message( + &fake_store, + "suche mir im internet rezepte für einen Apfelstrudel" + )); + } + + #[test] + fn allow_brave_when_skill_requires_and_substring_matches() { + let tmp = tempdir().unwrap(); + let fake_store = tmp.path().join("connection.json"); + let md = "---\nname: t\ndescription: d\ntags: [gov]\nrequires: [brave_web_search]\nbrave_allow_substrings: [widgets]\n---\n\nbody\n"; + write_custom_skill(&fake_store, "t", md).unwrap(); + assert!(!allow_brave_web_search_for_message( + &fake_store, + "hello world" + )); + assert!(allow_brave_web_search_for_message( + &fake_store, + "tell me about widgets" + )); + } + + #[test] + fn allow_brave_not_enabled_by_generic_news_tag() { + let tmp = tempdir().unwrap(); + let fake_store = tmp.path().join("connection.json"); + let md = "---\nname: t\ndescription: d\ntags: [news, gaming]\nrequires: [brave_web_search]\n---\n\nbody\n"; + write_custom_skill(&fake_store, "t", md).unwrap(); + assert!(!allow_brave_web_search_for_message( + &fake_store, + "gameinformer news" + )); + } + + #[test] + fn suche_nach_requires_web_context() { + let tmp = tempdir().unwrap(); + let fake_store = tmp.path().join("connection.json"); + assert!(!allow_brave_web_search_for_message( + &fake_store, + "suche nach gameinformer" + )); + assert!(allow_brave_web_search_for_message( + &fake_store, + "suche nach gameinformer im internet" + )); + } } diff --git a/src-tauri/src/modules/skills/types.rs b/src-tauri/src/modules/skills/types.rs index 5f0ea8d..7b10aa5 100644 --- a/src-tauri/src/modules/skills/types.rs +++ b/src-tauri/src/modules/skills/types.rs @@ -30,6 +30,10 @@ pub struct Skill { pub license: Option, #[serde(default)] pub requires: Vec, + /// If `requires` lists `brave_web_search`, optional substrings (case-insensitive) that must + /// appear in the user message before that tool is exposed — in addition to `tags` (length ≥4). + #[serde(default)] + pub brave_allow_substrings: Vec, #[serde(default)] pub origin: SkillOrigin, /// Optional extra rules from `mandatory.md` next to `SKILL.md` (server-only; not serialized to clients). diff --git a/src-tauri/src/modules/tool_engine/service.rs b/src-tauri/src/modules/tool_engine/service.rs index 1f67738..38fc642 100644 --- a/src-tauri/src/modules/tool_engine/service.rs +++ b/src-tauri/src/modules/tool_engine/service.rs @@ -30,6 +30,41 @@ const TE_CUSTOM_PREFIX: &str = "te_custom_"; /// In-image workspace stub when no host folders are mounted yet. pub const EMPTY_WORKSPACE_CONTAINER_ROOT: &str = "/tmp"; +fn filter_stored_catalog_passthrough( + entry: &ToolEntry, + stored: &HashMap, +) -> HashMap { + let mut out = HashMap::new(); + for name in &entry.passthrough_env { + if let Some(v) = stored.get(name) { + if !v.trim().is_empty() { + out.insert(name.clone(), v.clone()); + } + } + } + out +} + +/// Values for `podman|docker run --env=…`: `mcp.json` `catalog_passthrough` first, else host `std::env`. +fn merged_passthrough_for_container( + entry: &ToolEntry, + stored: &HashMap, +) -> HashMap { + let filtered = filter_stored_catalog_passthrough(entry, stored); + let mut out = HashMap::new(); + for name in &entry.passthrough_env { + if let Some(v) = filtered.get(name) { + out.insert(name.clone(), v.clone()); + } else if let Ok(v) = std::env::var(name) { + let t = v.trim().to_string(); + if !t.is_empty() { + out.insert(name.clone(), t); + } + } + } + out +} + /// Parse and validate a catalog JSON string. Returns `None` if parsing fails /// or schema_version is unsupported. fn parse_catalog(json: &str) -> Option { @@ -55,27 +90,61 @@ pub fn load_embedded_catalog() -> Result { .map_err(|e| format!("parse embedded mcp-tools.json: {e}")) } -/// Local `tools/mcp-tools.json` next to the crate or executable; with `debug_assertions` or -/// `LOCAL_TOOLS_CATALOG=1`, also walks up to 8 parents from `current_dir` (e.g. `tauri dev`). +fn push_unique_path(paths: &mut Vec, p: PathBuf) { + if paths.iter().any(|q| q == &p) { + return; + } + paths.push(p); +} + +/// True when we should walk `current_dir` for `tools/mcp-tools.json` and try those paths **before** +/// the compile-time `CARGO_MANIFEST_DIR` location (so `tauri dev` / dev builds use the repo you are in). +fn prefer_repo_catalog_paths_first() -> bool { + if cfg!(debug_assertions) { + return true; + } + if std::env::var("LOCAL_TOOLS_CATALOG") + .map(|v| { + let v = v.trim(); + v.eq_ignore_ascii_case("true") || v == "1" + }) + .unwrap_or(false) + { + return true; + } + std::env::var("TAURI_ENV") + .map(|v| v.trim().eq_ignore_ascii_case("development")) + .unwrap_or(false) +} + +/// Local `tools/mcp-tools.json`: in dev, cwd / ancestors first, then crate-relative path; in release, +/// crate-relative then next to the executable. `LOCAL_TOOLS_CATALOG=1` opts into the dev search in release too. fn try_load_local_tools_catalog() -> Option { let mut paths: Vec = Vec::new(); - paths.push(PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../tools/mcp-tools.json")); + let dev_order = prefer_repo_catalog_paths_first(); + if dev_order { + if let Ok(mut cwd) = std::env::current_dir() { + for _ in 0..8 { + push_unique_path(&mut paths, cwd.join("tools/mcp-tools.json")); + if !cwd.pop() { + break; + } + } + } + } + push_unique_path( + &mut paths, + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../tools/mcp-tools.json"), + ); if let Ok(exe) = std::env::current_exe() { if let Some(dir) = exe.parent() { - paths.push(dir.join("tools/mcp-tools.json")); + push_unique_path(&mut paths, dir.join("tools/mcp-tools.json")); } } - let ancestor_walk = cfg!(debug_assertions) - || std::env::var("LOCAL_TOOLS_CATALOG") - .map(|v| { - let v = v.trim(); - v.eq_ignore_ascii_case("true") || v == "1" - }) - .unwrap_or(false); - if ancestor_walk { + if !dev_order { if let Ok(mut cwd) = std::env::current_dir() { for _ in 0..8 { - paths.push(cwd.join("tools/mcp-tools.json")); + push_unique_path(&mut paths, cwd.join("tools/mcp-tools.json")); if !cwd.pop() { break; } @@ -117,7 +186,7 @@ fn try_load_local_tools_catalog() -> Option { None } -/// Local file, then remote URL, then embedded `mcp-tools.json`. +/// Local `tools/mcp-tools.json` (see [`try_load_local_tools_catalog`]), then remote URL, then embedded `mcp-tools.json`. pub async fn load_catalog() -> Result { if let Some(cat) = try_load_local_tools_catalog() { log::info!( @@ -213,6 +282,7 @@ fn catalog_tool_stdio_eq(a: &ServerEntry, b: &ServerEntry) -> bool { env: e1, direct_return: d1, private_host_path: p1, + catalog_passthrough: t1, }, ServerEntry::Stdio { command: c2, @@ -220,15 +290,16 @@ fn catalog_tool_stdio_eq(a: &ServerEntry, b: &ServerEntry) -> bool { env: e2, direct_return: d2, private_host_path: p2, + catalog_passthrough: t2, }, - ) => c1 == c2 && a1 == a2 && e1 == e2 && d1 == d2 && p1 == p2, + ) => c1 == c2 && a1 == a2 && e1 == e2 && d1 == d2 && p1 == p2 && t1 == t2, _ => false, } } /// Rebuild argv for one installed catalog tool from `mcp.json` + catalog entry. -/// The container env is baked into argv via `-e` flags, so `ServerEntry.env` stays empty -/// (host-process env does not propagate into the container). +/// Container env for catalog tools is baked into argv via `podman run --env=…`. User-provided +/// secrets live in `catalog_passthrough`; `env` is for legacy stdio (e.g. `npx`) only. fn rebuild_installed_catalog_tool_stdio( entry: &ToolEntry, host_paths: &[String], @@ -240,6 +311,7 @@ fn rebuild_installed_catalog_tool_stdio( command, direct_return, private_host_path, + catalog_passthrough, .. } = prev else { @@ -264,7 +336,9 @@ fn rebuild_installed_catalog_tool_stdio( _ => None, }; - let args = podman_run_argv_for_tool(entry, host_paths, private_bind.as_ref())?; + let stored = filter_stored_catalog_passthrough(entry, catalog_passthrough); + let merged = merged_passthrough_for_container(entry, &stored); + let args = podman_run_argv_for_tool(entry, host_paths, private_bind.as_ref(), &merged)?; Ok(Some(ServerEntry::Stdio { command: command.clone(), @@ -272,6 +346,7 @@ fn rebuild_installed_catalog_tool_stdio( env: HashMap::new(), direct_return: *direct_return, private_host_path: private_host_path.clone(), + catalog_passthrough: stored, })) } @@ -322,6 +397,7 @@ pub fn podman_run_argv_for_tool( entry: &ToolEntry, host_paths: &[String], private_bind: Option<&PrivateBind<'_>>, + passthrough: &HashMap, ) -> Result, String> { if entry.append_workspace_roots && !entry.mount_workspace { return Err("catalog: append_workspace_roots requires mount_workspace".into()); @@ -375,6 +451,10 @@ pub fn podman_run_argv_for_tool( args.push(format!("--env={k}={v}")); } + for (name, value) in passthrough { + args.push(format!("--env={name}={value}")); + } + args.push(image_ref); args.extend(entry.mcp_server_cmd.iter().cloned()); if entry.ignore_robots_txt { @@ -445,6 +525,45 @@ async fn image_present(runtime: &RuntimeInfo, image: &str) -> bool { /// A callback for streaming log lines during long-running operations. pub type LogFn = Box; +/// Last path segment of an OCI repository (e.g. `ghcr.io/org/pengine-brave-search` → `pengine-brave-search`). +fn oci_image_repository_basename(image: &str) -> Option { + let tail = image.rsplit('/').next()?.trim(); + if tail.is_empty() { + None + } else { + Some(tail.to_string()) + } +} + +/// `pengine/brave-search` → `brave-search` (matches `tools//Dockerfile` in this repo). +fn tool_repo_subdir(tool_id: &str) -> Option<&str> { + tool_id.rsplit('/').next().filter(|s| !s.is_empty()) +} + +/// Local tags users get when they follow `podman build -t : …`. +fn local_build_image_candidates(entry: &ToolEntry) -> Vec { + let Some(base) = oci_image_repository_basename(&entry.image) else { + return Vec::new(); + }; + let v = entry.current.as_str(); + vec![format!("{base}:{v}"), format!("localhost/{base}:{v}")] +} + +fn local_build_hint(entry: &ToolEntry) -> String { + let base = + oci_image_repository_basename(&entry.image).unwrap_or_else(|| "".into()); + let sub = tool_repo_subdir(&entry.id).unwrap_or(""); + let v = entry.current.as_str(); + let expected = format!("{}:{}", entry.image.trim(), v); + format!( + "No registry image for this version yet. Build locally, then retry install:\n podman build -t {base}:{v} -f tools/{sub}/Dockerfile tools/{sub}/\nIf the build succeeds, install tags that image as `{expected}` automatically.", + base = base, + sub = sub, + v = v, + expected = expected, + ) +} + /// Pull if missing; accept a local image when the digest is not pinned. Logs are prefixed with `[tool_id]`. async fn ensure_tool_image( runtime: &RuntimeInfo, @@ -472,12 +591,31 @@ async fn ensure_tool_image( log(&format!("{tag} pull failed but image found locally")); return Ok(()); } + if !pinned { + for alt in local_build_image_candidates(entry) { + if !image_present(runtime, &alt).await { + continue; + } + let ok = tokio::process::Command::new(&runtime.binary) + .args(["tag", &alt, &image_ref]) + .status() + .await + .map(|s| s.success()) + .unwrap_or(false); + if ok && image_present(runtime, &image_ref).await { + log(&format!( + "{tag} registry pull failed; tagged local `{alt}` as `{image_ref}`." + )); + return Ok(()); + } + } + } let hint = if pinned { - "Ensure the image is published to the registry." + "Ensure the image is published to the registry.".to_string() } else { - "No registry image yet. Build locally: podman build -t : tools//" + local_build_hint(entry) }; - return Err(format!("failed to pull image `{image_ref}` — {e}. {hint}")); + return Err(format!("failed to pull image `{image_ref}` — {e}\n{hint}")); } } @@ -620,7 +758,27 @@ pub async fn install_tool( _ => None, }; - let args = podman_run_argv_for_tool(entry, &host_paths, private_bind.as_ref())?; + let key = server_key(tool_id); + let stored = cfg + .servers + .get(&key) + .and_then(|e| { + if let ServerEntry::Stdio { + catalog_passthrough, + .. + } = e + { + Some(filter_stored_catalog_passthrough( + entry, + catalog_passthrough, + )) + } else { + None + } + }) + .unwrap_or_default(); + let merged = merged_passthrough_for_container(entry, &stored); + let args = podman_run_argv_for_tool(entry, &host_paths, private_bind.as_ref(), &merged)?; let server_entry = ServerEntry::Stdio { command: runtime.binary.clone(), @@ -628,9 +786,10 @@ pub async fn install_tool( env: HashMap::new(), direct_return: entry.direct_return, private_host_path: None, + catalog_passthrough: stored, }; - cfg.servers.insert(server_key(tool_id), server_entry); + cfg.servers.insert(key, server_entry); mcp_service::save_config(mcp_config_path, &cfg)?; Ok(()) @@ -869,6 +1028,7 @@ pub async fn add_custom_tool( env: HashMap::new(), direct_return: entry.direct_return, private_host_path: None, + catalog_passthrough: HashMap::new(), }; cfg.servers @@ -916,6 +1076,7 @@ pub fn sync_custom_tools_if_installed(cfg: &mut McpConfig, host_paths: &[String] env, direct_return, private_host_path, + catalog_passthrough, }) = cfg.servers.get(&key) else { continue; @@ -932,6 +1093,7 @@ pub fn sync_custom_tools_if_installed(cfg: &mut McpConfig, host_paths: &[String] env: env.clone(), direct_return: *direct_return, private_host_path: private_host_path.clone(), + catalog_passthrough: catalog_passthrough.clone(), }; cfg.servers.insert(key, new_entry); changed = true; @@ -942,6 +1104,7 @@ pub fn sync_custom_tools_if_installed(cfg: &mut McpConfig, host_paths: &[String] #[cfg(test)] mod tests { use super::*; + use std::collections::HashMap; use tempfile::tempdir; #[test] @@ -1009,7 +1172,7 @@ mod tests { .find(|v| v.version == fm.current) .unwrap(); assert_eq!(ver.digest, "sha256:placeholder"); - let argv = podman_run_argv_for_tool(fm, &[], None).expect("argv"); + let argv = podman_run_argv_for_tool(fm, &[], None, &HashMap::new()).expect("argv"); let tagged = format!("{}:{}", fm.image, fm.current); let image_ref = argv .iter() @@ -1042,7 +1205,7 @@ mod tests { config: pf, bot_id: "12345", }; - let argv = podman_run_argv_for_tool(mem, &[], Some(&pb)).expect("argv"); + let argv = podman_run_argv_for_tool(mem, &[], Some(&pb), &HashMap::new()).expect("argv"); let want_mount = format!( "-v={}:/mcp/data:rw", @@ -1059,4 +1222,28 @@ mod tests { "missing -e flag: argv={argv:?}" ); } + + #[test] + fn oci_image_repository_basename_last_segment() { + assert_eq!( + oci_image_repository_basename("ghcr.io/pengine-ai/pengine-brave-search").as_deref(), + Some("pengine-brave-search") + ); + } + + #[test] + fn local_build_image_candidates_use_catalog_tag() { + let catalog = load_embedded_catalog().unwrap(); + let brave = catalog + .tools + .iter() + .find(|t| t.id == "pengine/brave-search") + .expect("brave-search in catalog"); + let c = local_build_image_candidates(brave); + assert!( + c.contains(&"pengine-brave-search:0.1.0".to_string()) + && c.contains(&"localhost/pengine-brave-search:0.1.0".to_string()), + "{c:?}" + ); + } } diff --git a/src-tauri/src/modules/tool_engine/types.rs b/src-tauri/src/modules/tool_engine/types.rs index 3d020dd..78fe00c 100644 --- a/src-tauri/src/modules/tool_engine/types.rs +++ b/src-tauri/src/modules/tool_engine/types.rs @@ -103,7 +103,8 @@ pub struct ToolEntry { /// Resource limits applied to the container. #[serde(default)] pub limits: ResourceLimits, - /// When true, tool results go directly to the user without model summarisation. + /// When true, tool results may go straight to the user without a model pass (host may still + /// override for specific tools). Default false; `pengine/fetch` ships with false so replies stay human-readable. #[serde(default)] pub direct_return: bool, /// When set, image build (`tools-publish.yml`) installs this npm package at this version. @@ -120,6 +121,11 @@ pub struct ToolEntry { /// path via env so the tool can persist state (e.g. the Memory server's knowledge-graph JSON). #[serde(default, skip_serializing_if = "Option::is_none")] pub private_folder: Option, + /// Host env var names to forward into the container via `--env=KEY=VALUE`. Missing host + /// vars are silently skipped (the container is expected to surface its own error). Used + /// for required secrets like `BRAVE_API_KEY` that must reach the MCP server at runtime. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub passthrough_env: Vec, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src-tauri/src/shared/text.rs b/src-tauri/src/shared/text.rs index cb6760f..f8f1179 100644 --- a/src-tauri/src/shared/text.rs +++ b/src-tauri/src/shared/text.rs @@ -1,4 +1,13 @@ //! Small text helpers shared across modules. +//! +//! ## Reasoning vs user-visible text (defense in depth) +//! +//! Major hosted APIs (e.g. OpenAI reasoning models) keep chain-of-thought off the user-visible +//! channel entirely. Ollama does the same when thinking mode is enabled: traces go in +//! `message.thinking` and the host must drop that field before history or UI (see +//! `extract_message` in `ollama/service.rs`). Models still sometimes emit planning in +//! `message.content`; we strip tags, optional ``, JSON `reply`, heuristics, +//! and (when safe) constrain generations with a JSON schema so only `reply` reaches users. /// Remove reasoning scaffolding from model content. /// @@ -52,28 +61,108 @@ Example: scan forecastMorgen in X: k /// Injected as an extra system message immediately before the post-tool model call only. pub const PENGINE_POST_TOOL_REMINDER: &str = "\ You have tool output. Respond in the user's language. REQUIRED: put ONLY the user-visible answer inside \ -.... Put any English reasoning ONLY inside .... \ -Do not write plain paragraphs outside those tags."; +.... Put any English or meta reasoning ONLY inside .... \ +Do not narrate tool usage, skills, or planning in plain text; no sentences outside those tags. \ +If several `fetch` results are present, some may show robots.txt or User-Agent blocks — still use any successful excerpts; do not tell the user that nothing could be retrieved when other blocks contain usable text."; fn looks_like_english_scratchpad(s: &str) -> bool { s.contains("Okay, let's") || s.contains("Okay, let me") || s.contains("The user asked") + || s.contains("The user is asking") || s.contains("Wait, the user") || s.contains("the user's query") || s.contains("Let me check") || s.contains("I need to check") + || s.contains("First, I need") + || s.contains("according to the skill") + || s.contains("The instructions say") || s.matches("Wait,").count() >= 2 } +/// German / mixed prompts often produce German meta ("Zunächst muss …") without the English cues above. +fn looks_like_german_scratchpad(s: &str) -> bool { + let l = s.to_lowercase(); + l.contains("der nutzer fragt") + || l.contains("die nutzerin fragt") + || l.contains("zunächst muss") + || l.contains("zuerst muss ich") + || l.contains("ich sollte jetzt") + || (l.contains(" laut skill") || l.contains("laut der skill")) +} + +fn looks_like_scratchpad_meta(s: &str) -> bool { + looks_like_english_scratchpad(s) || looks_like_german_scratchpad(s) +} + +fn paragraph_opens_like_meta(p: &str) -> bool { + let head: String = p.chars().take(120).collect::().to_lowercase(); + head.starts_with("okay,") + || head.starts_with("wait,") + || head.starts_with("let me ") + || head.starts_with("first,") + || head.starts_with("hmm,") + || head.contains("the user asked") + || head.contains("the user is asking") + || head.contains("der nutzer fragt") + || head.contains("die nutzerin fragt") +} + +fn last_non_meta_paragraph(s: &str) -> Option { + for block in s.rsplit("\n\n") { + let t = block.trim(); + if t.len() < 60 { + continue; + } + if paragraph_opens_like_meta(t) { + continue; + } + let open: String = t.chars().take(220).collect::().to_lowercase(); + if open.contains("the user is") + || open.contains("i should now") + || open.contains("according to the search") + || open.contains("tool response") + { + continue; + } + return Some(t.to_string()); + } + None +} + +/// If the model inlined a labeled answer section, keep only that tail (exact-case markers common in DE output). +fn strip_after_inline_answer_label(s: &str) -> Option { + for marker in [ + "**Antwort:**", + "**Antwort**", + "Antwort:\n\n", + "\nAntwort:\n", + "**Zusammenfassung:**", + "**Zusammenfassung**", + ] { + if let Some(idx) = s.find(marker) { + let tail = + s[idx + marker.len()..].trim_start_matches(|c: char| c == ':' || c.is_whitespace()); + if tail.len() >= 12 { + return Some(tail.to_string()); + } + } + } + None +} + /// When the model ignores `` but dumps English chain-of-thought, keep the tail that /// usually starts the real answer. Used only as a last resort after tag parsing fails. fn strip_plain_scratchpad_fallback(s: &str) -> String { let s = s.trim(); - if s.is_empty() || !looks_like_english_scratchpad(s) { + if s.is_empty() || !looks_like_scratchpad_meta(s) { return s.to_string(); } + if let Some(tail) = strip_after_inline_answer_label(s) { + return tail; + } + let markers = [ "\n\nMorgen (", "\nMorgen (", @@ -86,6 +175,17 @@ fn strip_plain_scratchpad_fallback(s: &str) -> String { "\n\nTomorrow in ", "\nTomorrow in ", "\n\nThe answer is ", + "\n\n### Antwort", + "\n\nZusammenfassung:", + "\nZusammenfassung:", + "\n\n**Zusammenfassung", + "\n\nKurzfassung:", + "\nKurzfassung:", + "\n\nDamit:", + "\n\nFazit:", + "\n\nZusammenfassend", + "\n\nKurz gesagt,", + "\nKurz gesagt,", ]; let mut best: Option = None; for m in markers { @@ -110,7 +210,12 @@ fn strip_plain_scratchpad_fallback(s: &str) -> String { return s[i..].trim_start().to_string(); } - s.to_string() + if let Some(p) = last_non_meta_paragraph(s) { + return p; + } + + // Meta-only or no recoverable user-facing block — better empty than leaking CoT to Telegram/UI. + String::new() } /// Drop paired XML/HTML-style blocks iteratively. @@ -175,20 +280,39 @@ fn parse_json_reply_field(content: &str) -> Option { pub fn normalize_assistant_message_content(raw: &str, json_object_reply: bool) -> String { if json_object_reply { if let Some(reply) = parse_json_reply_field(raw) { - return reply; + return block_if_still_meta_scratchpad(reply); } } let s = strip_think(raw); let s = strip_tag_pair(&s, concat!("<", "think", ">"), concat!("")); + // Explicit host contract wins; do not second-guess tagged user text. if let Some(inner) = last_tag_inner(&s, "pengine_reply") { - return inner; + return inner.trim().to_string(); + } + + // Some reasoning models wrap only the final reply in `…`. + if let Some(inner) = last_tag_inner(&s, "answer") { + return block_if_still_meta_scratchpad(inner); } let without_plan = strip_all_named_blocks(&s, "pengine_plan"); let t = without_plan.trim().to_string(); - strip_plain_scratchpad_fallback(&t) + let out = strip_plain_scratchpad_fallback(&t); + block_if_still_meta_scratchpad(out) +} + +/// If the model slipped planning into the only channel we show, drop it rather than leak CoT. +fn block_if_still_meta_scratchpad(s: String) -> String { + let t = s.trim(); + if t.is_empty() { + return String::new(); + } + if looks_like_scratchpad_meta(t) { + return String::new(); + } + t.to_string() } /// Strip ANSI escape sequences (CSI + OSC) and collapse runs of blank lines. @@ -405,6 +529,18 @@ mod tests { assert_eq!(normalize_assistant_message_content(s, false), "ok"); } + #[test] + fn normalize_assistant_extracts_answer_tag_when_no_pengine_reply() { + let s = "rOnly this."; + assert_eq!(normalize_assistant_message_content(s, false), "Only this."); + } + + #[test] + fn normalize_assistant_json_reply_still_meta_gets_cleared() { + let s = r#"{"reply":"Okay, let's see. The user asked for X."}"#; + assert_eq!(normalize_assistant_message_content(s, true), ""); + } + #[test] fn normalize_assistant_fallback_strips_english_scratchpad_before_morgen() { let pad = "Okay, let's see. The user asked for weather.\n\n"; @@ -416,6 +552,34 @@ mod tests { ); } + #[test] + fn normalize_assistant_fallback_strips_after_antwort_label() { + let pad = "Okay, let's see. The user is asking about divorce.\n\n"; + let answer = "**Antwort:** Ja, das ist möglich."; + let combined = format!("{pad}{answer}"); + assert_eq!( + normalize_assistant_message_content(&combined, false), + "Ja, das ist möglich." + ); + } + + #[test] + fn normalize_assistant_fallback_meta_only_returns_empty() { + let s = "Okay, let's see. The user is asking about X. First, I need to check. Wait, the skill says. So I should."; + assert_eq!(normalize_assistant_message_content(s, false), ""); + } + + #[test] + fn normalize_assistant_fallback_german_meta_then_paragraph() { + let pad = "Zunächst muss ich die Frage prüfen.\n\n"; + let answer = "Kurz: In Österreich gilt aus den Familienberatungsstellen-Infos Folgendes für Ihren Fall und die Beratungsbestätigung."; + let combined = format!("{pad}{answer}"); + assert_eq!( + normalize_assistant_message_content(&combined, false), + answer.trim() + ); + } + #[test] fn compact_tool_output_strips_ansi_csi() { let s = "\x1b[31mred\x1b[0m normal"; diff --git a/src/modules/mcp/components/AddServerForm.tsx b/src/modules/mcp/components/AddServerForm.tsx index 5fd014d..e01de2e 100644 --- a/src/modules/mcp/components/AddServerForm.tsx +++ b/src/modules/mcp/components/AddServerForm.tsx @@ -1,5 +1,6 @@ import { useState } from "react"; import type { ServerEntry } from ".."; +import { buildEnvMapFromMcpForm } from "../mcpEnvHelpers"; type Props = { busy: boolean; @@ -24,7 +25,9 @@ export function AddServerForm({ busy, onAdd }: Props) { const [nativeId, setNativeId] = useState(""); const [command, setCommand] = useState(""); const [argsText, setArgsText] = useState(""); - const [envText, setEnvText] = useState(""); + const [apiKeyName, setApiKeyName] = useState(""); + const [apiKeyValue, setApiKeyValue] = useState(""); + const [envOtherLines, setEnvOtherLines] = useState(""); const [directReturn, setDirectReturn] = useState(false); const reset = () => { @@ -35,7 +38,9 @@ export function AddServerForm({ busy, onAdd }: Props) { setManualKind("stdio"); setCommand(""); setArgsText(""); - setEnvText(""); + setApiKeyName(""); + setApiKeyValue(""); + setEnvOtherLines(""); setDirectReturn(false); setError(null); }; @@ -127,11 +132,13 @@ export function AddServerForm({ busy, onAdd }: Props) { .split("\n") .map((l) => l.trim()) .filter(Boolean); - const env: Record = {}; - for (const line of envText.split("\n")) { - const eq = line.indexOf("="); - if (eq > 0) env[line.slice(0, eq).trim()] = line.slice(eq + 1).trim(); - } + const env = buildEnvMapFromMcpForm({ + otherLinesText: envOtherLines, + apiKeyName, + apiKeyValue, + preservedSecretValue: null, + replacingSecret: true, + }); try { await onAdd(name, { @@ -328,15 +335,50 @@ export function AddServerForm({ busy, onAdd }: Props) { className={`${inputClass} resize-y`} /> +
+ +

+ Use a dedicated password field instead of putting secrets in the multi-line env + box. +

+
+
+ + setApiKeyName(e.target.value)} + placeholder="BRAVE_API_KEY" + autoComplete="off" + className={inputClass} + /> +
+
+ + setApiKeyValue(e.target.value)} + placeholder="Secret value" + autoComplete="new-password" + className={inputClass} + /> +
+
+