35 feature add ai model token count#39
Merged
Merged
Conversation
- Added support for browsing and installing skills and plugins from ClawHub through a new ClawHubBrowse component. - Updated the skills API to include new search filters and improved error handling for ClawHub queries. - Introduced new types for ClawHub plugins and enhanced the skills types to accommodate additional metadata. - Integrated Radix UI's scroll area for better user experience in the skills browsing interface. - Updated package dependencies to include new Radix UI components.
- Introduced a new `ToolMetadata` module to classify tools with inferred categories and risk levels for improved routing and observability. - Updated the `ToolDef` structure to include `category` and `risk` fields, enhancing the tool definition with additional metadata. - Enhanced the tool selection process to utilize recent tool usage and memory server context for better decision-making during model turns. - Implemented logging for tool context selection latency to monitor performance and optimize routing decisions. - Refactored the agent's chat options to better manage completion tokens and temperature settings based on tool usage.
- Updated the tool selection process to incorporate chat session recording status, allowing for more context-aware tool routing. - Refactored the `select_tools_for_turn` function to include a new parameter for chat session recording, improving decision-making based on user interactions. - Enhanced logging to include chat session recording information, aiding in performance monitoring and debugging. - Introduced new utility functions to determine the relevance of memory tools based on user messages and recent tool usage, optimizing memory API calls during interactions.
1 task
📝 WalkthroughWalkthroughIntroduces a keyword-matching framework, per-turn think-mode and tool-routing in the bot agent, MCP tool metadata/risk classification and selection APIs, ClawHub plugins search + frontend browsing, message normalization/chunking utilities, SKILL.md adoption, and adds related Rust/npm dependencies and HTTP/Tauri endpoints. Changes
Sequence DiagramsequenceDiagram
actor User
participant Agent
participant Registry
participant KeywordModule
participant Ollama
participant Tools
User->>Agent: send message (maybe with /think)
Agent->>KeywordModule: detect keywords (THINK_ON, etc.)
Agent->>Registry: select_tools_for_turn(message, recent_tools)
Registry-->>Agent: ToolContextSelection (tools_json, select_ms, routing)
Agent->>Ollama: chat_with_tools(messages, tools_json, ChatOptions{think:...})
alt Ollama returns tool calls
Agent->>Tools: invoke tools
Tools-->>Agent: tool outputs
Agent->>Agent: compact_tool_output(), truncate_for_model()
Agent->>Ollama: follow-up chat_with_tools(updated_messages, ChatOptions{think:false})
else no tool calls and no content
Agent->>Registry: full_tool_context() (escalate)
Registry-->>Agent: full tool catalog
Agent->>Ollama: chat_with_tools(messages, full_tools, ChatOptions{think:false})
end
Agent->>Agent: normalize_assistant_message_content(), strip_think
Agent-->>User: final reply
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (14)
src-tauri/src/modules/skills/types.rs (1)
106-112: Considerrename_all = "camelCase"for consistency.Other new structs in this file (
ClawHubSkill,ClawHubPluginSummary) use#[serde(rename_all = "camelCase")], whileClawHubPluginsResponseuses an explicitrename = "nextCursor"on the only camelCase field. Switching torename_allkeeps the module stylistically uniform and avoids forgetting a rename if new camelCase fields are added later.♻️ Proposed tweak
#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] pub struct ClawHubPluginsResponse { #[serde(default)] pub items: Vec<ClawHubPluginSummary>, - #[serde(default, rename = "nextCursor")] + #[serde(default)] pub next_cursor: Option<String>, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/skills/types.rs` around lines 106 - 112, ClawHubPluginsResponse currently uses an explicit rename on next_cursor while other structs use #[serde(rename_all = "camelCase")]; change the struct attribute to #[serde(rename_all = "camelCase")] on ClawHubPluginsResponse and remove the per-field rename (keep #[serde(default)] on items and next_cursor) so new camelCase fields are handled consistently and you don't need individual renames for next_cursor.src/modules/skills/api/index.ts (2)
132-133: Minor inconsistency betweensearchClawHubandsearchClawHubPluginslimit handling.Here
limitis only sent whenfilters.limitis explicitly provided and positive, but insearchClawHubPlugins(line 164)limitis always sent using the resolvedlim(defaulting to 30). The timeout calculation at line 122 already uses the resolvedlim, so the two code paths quietly disagree on what the effective limit is. Consider aligning both functions — either always send the resolvedlim, or compute the timeout from the rawfilters.limit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/skills/api/index.ts` around lines 132 - 133, The two functions searchClawHub and searchClawHubPlugins disagree on limit handling; compute a single resolved limit value (e.g., const lim = filters.limit && filters.limit > 0 ? filters.limit : 30) up front and use that resolved lim both for timeout calculation and to set the URL param (replace the conditional url.searchParams.set("limit", String(filters.limit)) with url.searchParams.set("limit", String(lim))). Ensure both functions use the same resolved variable name (lim) so the timeout and request param are consistent.
127-128: Implicittruedefaults forhighlighted/nonSuspiciousmay surprise callers.Because of
String(filters.highlighted !== false), omitting these filters sendshighlighted=trueandnonSuspicious=trueto the backend — i.e. the frontend silently filters results unless the caller explicitly passesfalse. If that is intentional, it's worth a docstring onClawHubSearchFilterssimilar to the one onenrich. Otherwise, consider only setting these params when explicitly provided (matching the handling ofstaffPicks,cleanOnly,tag).♻️ Alternative: only send when explicitly set
- url.searchParams.set("highlighted", String(filters.highlighted !== false)); - url.searchParams.set("nonSuspicious", String(filters.nonSuspicious !== false)); + if (filters.highlighted !== undefined) + url.searchParams.set("highlighted", String(filters.highlighted)); + if (filters.nonSuspicious !== undefined) + url.searchParams.set("nonSuspicious", String(filters.nonSuspicious));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/skills/api/index.ts` around lines 127 - 128, The current code always sets url.searchParams for "highlighted" and "nonSuspicious" to true when the caller omits them (via String(filters.highlighted !== false)), which silently applies those filters; change this to only add those query params when the caller explicitly provides them (e.g., check filters.highlighted !== undefined / filters.highlighted !== null before calling url.searchParams.set for "highlighted", and do the same for filters.nonSuspicious) to match the handling used for staffPicks/cleanOnly/tag, and optionally add a docstring on the ClawHubSearchFilters type explaining default behavior if keeping implicit-true was intentional.src/modules/skills/components/ClawHubBrowse.tsx (2)
234-248: Observer reconnects on every list mutation.The effect's dep array includes
resultsandvisiblePlugins, so each appended page triggersobs.disconnect()+ a freshIntersectionObserver(...). Functionally fine but wasteful, and in theory creates a brief window where scrolling past the sentinel doesn't fire. SinceloadMoreRef.currentis stable per-tab render andloadMoreis already memoized, narrowing deps to[loadMore, clawRegistry](and relying on the sentinel's conditional render for hasMore changes) would be cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/skills/components/ClawHubBrowse.tsx` around lines 234 - 248, The IntersectionObserver effect in ClawHubBrowse is reconnecting on every list mutation because its dependency array includes changing arrays like results and visiblePlugins; update the useEffect to only depend on stable references (e.g., loadMore and clawRegistry) so it doesn't teardown/recreate the observer on each append, keep using listViewportRef.current and loadMoreRef.current inside the effect, and rely on the sentinel render logic (hasMoreSkills/hasMorePlugins) to control observation instead of including results/visiblePlugins in the deps.
88-107: AssigningqueryRef.current = queryin the render body.Line 104 mutates a ref during render. It works but violates React's "render should be pure" expectation and, under concurrent rendering / Strict Mode, the assignment can run for a render that is later thrown away. Prefer
useEffect(() => { queryRef.current = query; }, [query])or readquerydirectly in the callbacks via its state (and include it in theuseCallbackdeps).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/skills/components/ClawHubBrowse.tsx` around lines 88 - 107, The ref assignment queryRef.current = query is performed during render which breaks render purity; move that mutation into a useEffect like useEffect(() => { queryRef.current = query; }, [query]) or eliminate queryRef and read query state directly inside the callbacks (and add query to their useCallback dependency arrays) so the ref is only updated after render and callbacks use the correct up-to-date value; update any places referencing queryRef to reflect this change (in the ClawHubBrowse component and related handlers).src-tauri/src/modules/skills/service.rs (1)
588-600:"a"staff-picks fallback is a workable hack — document the upstream contract.Seeding an empty query with
"a"whenstaff_picksis on is a reasonable workaround for ClawHub returning[]for emptyq, but it silently biases results toward names/descriptions containing the letter "a". If staff-picks browsing becomes a primary UX (the default option), consider asking the ClawHub maintainers to allowq=withstaffPicks=true, or probing a small set of seed letters and merging/deduping. The current comment is good; a TODO link-back would make this easier to revisit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/skills/service.rs` around lines 588 - 600, The current hack in search_clawhub that substitutes "a" into effective_q when q is empty and opts.staff_picks is true should be made explicit and traceable: extract the fallback into a named constant (e.g., STAFF_PICKS_SEED = "a") and add a TODO comment above search_clawhub referencing an upstream issue/PR URL and describing the desired contract (allow q= with staffPicks=true or return seeded staff picks), and if staff_picks may become the primary UX add an implementation note (or simple improvement) to probe a small set of seed letters (e.g., ["a","e","i"]) and merge/dedupe results instead of a single-letter bias; ensure references to effective_q and ClawHubSearchOptions::staff_picks remain and update any tests or comments accordingly.src-tauri/src/shared/state.rs (1)
87-102: O(n) FIFO eviction — preferVecDeque.
recent_tool_namesandtool_ctx_latency_msare both used as bounded FIFOs but implemented withVec<_>andremove(0), which is O(n) per eviction. With caps of 32/128 the absolute cost is small, butVecDequewithpush_back/pop_frontmakes the intent clearer and is O(1). Same comment applies torecord_tool_selection_msat lines 104–109.♻️ Proposed refactor
- pub recent_tool_names: Arc<Mutex<Vec<String>>>, - pub tool_ctx_latency_ms: Arc<Mutex<Vec<u64>>>, + pub recent_tool_names: Arc<Mutex<VecDeque<String>>>, + pub tool_ctx_latency_ms: Arc<Mutex<VecDeque<u64>>>,With
push_back+pop_fronton overflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/shared/state.rs` around lines 87 - 102, Replace the FIFO Vec-based buffers with VecDeque to avoid O(n) remove(0) evictions: change the types of recent_tool_names and tool_ctx_latency_ms to VecDeque<String>/VecDeque<u64> (or appropriate element type), import std::collections::VecDeque, and update note_tools_used and record_tool_selection_ms to use push_back to append and pop_front while len() > RECENT_TOOLS_CAP / TOOL_CTX_LATENCY_CAP to evict oldest entries; keep existing trimming/validation logic (trim/empty checks) but push_back the trimmed strings and use pop_front for overflow.src-tauri/src/modules/mcp/tool_metadata.rs (1)
24-26: Ambiguous operator precedence — add parentheses.
name == "manage_tools" || desc.contains("uninstall") && desc.contains("catalog")parses asname == "manage_tools" || (desc.contains("uninstall") && desc.contains("catalog")), which appears to be the intent, but relying on&&binding tighter than||here is easy to misread and a common bug magnet. Parenthesize explicitly.♻️ Proposed fix
- if name == "manage_tools" || desc.contains("uninstall") && desc.contains("catalog") { + if name == "manage_tools" || (desc.contains("uninstall") && desc.contains("catalog")) { return ("system".into(), ToolRisk::High); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/mcp/tool_metadata.rs` around lines 24 - 26, Add explicit parentheses around the boolean expression to remove ambiguity: change the conditional that returns ("system".into(), ToolRisk::High) to use (name == "manage_tools") || (desc.contains("uninstall") && desc.contains("catalog")) so the intent is explicit; update the expression in the block where name and desc are checked (the conditional that currently reads name == "manage_tools" || desc.contains("uninstall") && desc.contains("catalog")).src-tauri/src/modules/keywords.rs (1)
10-18: Nit: consider a&'static [&'static KeywordGroup]slice instead of a freshVec.Allocation is trivial here (5 refs) and this isn't a hot path, but since the catalog is fully static, you could expose it as a
constor&'static [...]and avoid the heap entirely — plus callers could iterate without taking ownership. Entirely optional.pub const ALL_KEYWORD_GROUPS: &[&KeywordGroup] = &[ &memory::SESSION_START, &memory::SESSION_END, &memory::DIARY_START, &memory::DIARY_END, &ollama::keywords::THINK_ON, ];(If Tauri serialization needs a
Vec, the existing shape is fine — keep as-is.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/keywords.rs` around lines 10 - 18, Replace the heap-allocating function all_keyword_groups with a static slice to avoid allocating a Vec: introduce a pub const ALL_KEYWORD_GROUPS: &'static [&'static KeywordGroup] (or pub const ALL_KEYWORD_GROUPS: &[&KeywordGroup]) initialized with &memory::SESSION_START, &memory::SESSION_END, &memory::DIARY_START, &memory::DIARY_END, &ollama::keywords::THINK_ON, and update any callers of all_keyword_groups to reference ALL_KEYWORD_GROUPS (or provide a thin fn wrapper that returns ALL_KEYWORD_GROUPS) so callers can iterate without taking ownership.src-tauri/src/modules/mcp/client.rs (1)
105-124: Optional: consider aToolDefconstructor to DRY the three call sites.The
category: None, risk: ToolRisk::Lowboilerplate plustool_metadata::apply(&mut def)is repeated identically here and twice innative.rs(dice_named,tool_manager_named). A small helper (e.g.,ToolDef::new_with_metadata(server_name, name, description, schema, direct_return)) or aDefaultimpl for the metadata-derived fields would centralize the invariant that every newToolDefmust go throughapply.Not blocking — current code is correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/mcp/client.rs` around lines 105 - 124, Create a small constructor or helper for ToolDef that centralizes the repeated metadata initialization and apply call: add e.g. ToolDef::new_with_metadata(server_name: &str, name: String, description: Option<String>, input_schema: serde_json::Value, direct_return: bool) (or implement Default for the metadata fields) which constructs the ToolDef with category: None and risk: ToolRisk::Low and calls tool_metadata::apply(&mut def) before returning; then replace the inline blocks in client.rs (current anonymous block building ToolDef), native.rs dice_named and tool_manager_named with calls to ToolDef::new_with_metadata(...) to DRY the repeated boilerplate and ensure apply is always invoked.src-tauri/src/modules/mcp/registry.rs (2)
445-459: Always-on dedupe keys on name only, not (server, name).
push_always_on_toolsinserts intoseenusingtool.name.clone(). If two providers each expose a tool literally namedfetchortime(e.g. an MCP server that happens to use the same name as the native tool), only the first iterated one is promoted and the other is silently dropped. This is consistent with the rest ofToolRegistry(which already has ambiguity handling inresolve_tool), so not a new bug — just flagging because the routing path doesn't surface the ambiguity warning thatresolve_tooldoes. A log-warn whenALWAYS_ON_TOOL_NAMESmatches more than one (server, name) pair would help diagnose misrouting in that scenario.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/mcp/registry.rs` around lines 445 - 459, push_always_on_tools currently dedupes by tool.name only (seen.insert(tool.name.clone())), which drops tools from other providers that share the same name; change the dedupe key to include the provider/server plus name (e.g., combine tool.server and tool.name or use a tuple key) when inserting into seen so each (server, name) pair is unique, and add a log-warn when ALWAYS_ON_TOOL_NAMES matches more than one (server, name) pair for the same name to surface ambiguity (refer to ALWAYS_ON_TOOL_NAMES, ToolDef, push_always_on_tools, and the seen HashSet); keep existing resolve_tool ambiguity behavior intact.
244-257: Minor:full_tool_contexthardcodesselect_ms: 0.After a routing escalation (see
agent.rslines 606–616),state.record_tool_selection_ms(tool_ctx.select_ms)is not re-called, but the emit_log on the next iteration would show a staleselect_ms. The0here is defensible since the escalation is essentially free, but a follow-upemit_log("tool_ctx", "escalate full catalog …")already covers observability — so leaving as-is is fine. No action unless you want to track total routing time per turn in metrics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/mcp/registry.rs` around lines 244 - 257, full_tool_context currently hardcodes select_ms to 0 which leaves a stale selection time in logs after routing escalation; change ToolRegistry::full_tool_context to accept a select_ms: u64 parameter and assign that value to ToolContextSelection.select_ms, then update all call sites (e.g., where agent.rs constructs the tool context during escalation) to pass the actual recorded selection time (the value used with state.record_tool_selection_ms) so emit_log sees the correct select_ms; ensure the signature change is propagated and compiled across usages.src-tauri/src/modules/ollama/service.rs (1)
159-169: Minor: schema helper feels misplaced here.
summarize_reply_json_schema()encodes a caller contract (bot agent's phase-2 summarize pass) but lives in the transport layer. If another caller later wants a different schema, it ends up adding siblings here. Consider moving it tobot/agent.rs(or abot/schemas.rs) and letting the service expose onlyChatOptions.format: Option<Value>. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/ollama/service.rs` around lines 159 - 169, The summarize_reply_json_schema() helper belongs to the bot layer, not the transport/service; move this function into bot/agent.rs (or a new bot/schemas.rs) and import it where the agent builds phase-2 summaries, and update the service module to stop owning agent-specific schemas—leave the transport to expose only a generic ChatOptions.format: Option<serde_json::Value> (or similar) and have code that previously referenced summarize_reply_json_schema() point to the new location (adjust any use sites in modules that call summarize_reply_json_schema and remove the helper from service.rs).src-tauri/src/shared/text.rs (1)
69-114: Fallback is German-answer-specific; consider guarding its scope.
strip_plain_scratchpad_fallbackdepends on hardcoded tail markers (Morgen …,Heute in,Tomorrow in,The answer is) that are almost entirely German weather-reply shapes. This works as a last-resort bandage for the specific failure mode you've been debugging, but:
- It silently no-ops for non-German answers that follow the same scratchpad pattern (French, Spanish, JSON code fences, …), meaning the user still sees English scratchpad text.
- A future model fix that emits proper
<pengine_reply>tags will never exercise this path, so bit-rot risk is high.Non-blocking, but consider (a) logging when this fallback actually fires so you can measure whether it's still earning its keep, and (b) filing a note to revisit once the system prompt reliably produces
<pengine_reply>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/infrastructure/http_server.rs`:
- Around line 1268-1297: ClawHubSearchQuery’s snake_case fields (e.g.,
non_suspicious, staff_picks, clean_only) are not matching the frontend camelCase
URL params (nonSuspicious, staffPicks, cleanOnly); update the ClawHubSearchQuery
definition to accept camelCase by adding #[serde(rename_all = "camelCase")]
above the ClawHubSearchQuery struct (or alternatively add explicit
#[serde(rename = "nonSuspicious")] / #[serde(rename = "staffPicks")] /
#[serde(rename = "cleanOnly")] to the individual fields) so incoming query
parameters deserialize correctly.
In `@src-tauri/src/modules/bot/agent.rs`:
- Around line 599-617: The escalation to full catalog mistakenly treats the
retry as a post-tool step (using step to decide chat_options_for_agent_step and
inject_post_tool), so introduce a new counter (e.g., tool_rounds) initialized to
0 and use tool_rounds > 0 wherever the code currently uses step > 0 to decide
post-tool behavior (inject_post_tool, PENGINE_POST_TOOL_REMINDER and choosing
chat options), then increment tool_rounds only when actual tool-result messages
are appended (just before the existing return that checks if
!direct_replies.is_empty()), ensuring that the routing_escalated continue will
re-enter as a true step-0 call with the original think/num_predict behavior and
without the post-tool reminder.
In `@src-tauri/src/modules/bot/service.rs`:
- Around line 10-12: split_by_chars currently counts Rust .chars() (Unicode
scalar values) while TELEGRAM_CHUNK_BUDGET = 4000 assumes UTF‑16 units; this can
overflow Telegram's 4096 UTF‑16 limit for emoji/supplementary characters. Fix by
either reducing TELEGRAM_CHUNK_BUDGET to 2000, or (preferred) implement
split_by_utf16_units: change split_by_chars usage to a function that counts
UTF‑16 code units (string.encode_utf16().count or iterating utf16 units) and
chunk the text so each piece's UTF‑16 length <= TELEGRAM_CHUNK_BUDGET
(keep/rename constant if you retain 4000), updating all callers of
split_by_chars accordingly (look for split_by_chars and TELEGRAM_CHUNK_BUDGET in
service.rs).
In `@src-tauri/src/modules/mcp/registry.rs`:
- Around line 639-665: recent_tool_score currently gives highest weight to the
oldest entry because it uses recent.len().saturating_sub(i); change the weight
calculation to give newer indices larger weights (e.g., use weight = i + 1 or
compute relative age so later indices score higher) in the recent_tool_score
function; also update/add a doc comment on recent_tools_snapshot to state it
returns names in insertion order (oldest first) so callers know the slice is
oldest-to-newest.
In `@src-tauri/src/modules/mcp/tool_metadata.rs`:
- Around line 38-48: The classification incorrectly tags memory tools like
delete_observations and delete_relations as filesystem/High because the generic
name.contains("write"|"delete"|"remove"...) heuristic runs before the
memory-namespace check; update the logic in tool_metadata.rs so the memory-tool
matching (the matches! list that includes memory namespace checks) is evaluated
before the generic write/delete/remove contains checks (the name.contains(...)
block), ensuring tools in the memory namespace (e.g., delete_observations,
delete_relations) are classified by the memory-specific branch instead of
falling through to the filesystem High heuristic.
In `@src-tauri/src/modules/mcp/types.rs`:
- Around line 74-82: The serde rename_all on the ToolRisk enum is ineffective
because all ToolRisk fields on ToolDef are marked #[serde(skip)]; either remove
the unused attribute from the ToolRisk definition (delete #[serde(rename_all =
"lowercase")]) or, if you actually intended ToolRisk to be serialized in
ToolDef, remove the #[serde(skip)] from those ToolDef fields so ToolRisk's
serialization (lowercase names) is applied; update the ToolRisk enum or the
ToolDef field attributes accordingly.
In `@src-tauri/src/modules/ollama/keywords.rs`:
- Around line 9-17: The phrase "deep think" in the THINK_ON_EN constant causes
unintended substring matches under MatchMode::Substring (e.g., "deep thinker");
remove "deep think" from the THINK_ON_EN array (KEEP entries like "think deeply"
and other intended cues) or replace it with a less ambiguous token if you need
an additional cue, and keep MatchMode::Substring behavior in mind when editing
other entries like "be thorough".
In `@src-tauri/src/modules/skills/service.rs`:
- Around line 416-493: Add a clear comment near the regex statics
(SKILL_STATS_RE, OWNER_HANDLE_RE, SEMVER_RE) and the parsing functions
(skill_stats_re, owner_handle_re, semver_re, parse_openclaw_skill_html,
parse_stats_blob, extract_owner_before_skill) stating that these regexes and
string matches target an internal Next.js RSC serialization format and are
intentionally brittle: failures degrade silently (ParsedOpenclawSkillPage fields
are Option/bool, merge_openclaw_parsed only overwrites on Some, and
fetch_openclaw_skill_html returns None on error), so do not convert the
expect(...) calls into propagated errors; also note parse_stats_blob’s
simplistic split on ',' and ':' will ignore nested/quoted commas and is
acceptable for current flat numeric payloads but may miss future quoted values.
- Around line 529-552: The current enrich_clawhub_skills_from_openclaw spawns
one future per skill and uses join_all, causing unbounded concurrency; replace
that logic with a bounded stream using futures::stream::iter(...) and
.map(|slug| { let client = client.clone(); async move { let html =
fetch_openclaw_skill_html(&client, &slug).await?; let parsed =
parse_openclaw_skill_html(&html); Some((slug, parsed)) }
}).buffer_unordered(8).collect::<Vec<_>>().await (or similar) so at most ~8 HTTP
fetches are in-flight; then flatten/collect the results into the HashMap<String,
ParsedOpenclawSkillPage> and proceed to merge_openclaw_parsed as before. Ensure
you reference enrich_clawhub_skills_from_openclaw, fetch_openclaw_skill_html,
parse_openclaw_skill_html and merge_openclaw_parsed when making the change.
In `@src/modules/skills/components/ClawHubBrowse.tsx`:
- Around line 192-199: The client-side tag filter (visiblePlugins computed from
pluginResults and pluginTagFilter) can hide all loaded items so the
infinite-scroll sentinel disappears and further pages never auto-load; update
the rendering logic that places the sentinel/infinite-scroll trigger so it
renders when hasMorePlugins is true regardless of visiblePlugins.length (or show
a "Load more to search further" affordance when visiblePlugins is empty and
hasMorePlugins is true), and ensure the sentinel's container references
visiblePlugins, pluginResults, pluginTagFilter, and hasMorePlugins so users can
continue loading additional pages to search beyond the currently-loaded results.
- Around line 109-175: Both runSkillsSearch and runPluginsSearch can produce
stale overwrites when toggling filterCleanOnly, clawSort, or switching tabs
while a request is in flight; add a per-search generation guard or
AbortController so only the latest response updates state. Implement by keeping
a mutable ref (e.g., searchGenRef) that you increment before each new invocation
of runSkillsSearch/runPluginsSearch (or create/replace an AbortController and
pass its signal), capture the current token inside the async call, and before
calling setResults, setPluginResults, setHasMoreSkills, setHasMorePlugins,
setBrowseError, or changing pluginCursorRef.current verify the token matches the
latest ref (or that the AbortSignal is not aborted); cancel/ignore older
responses so in-flight completions cannot clobber newer UI state.
In `@tools/mcp-tools.json`:
- Line 25: The change makes pengine/file-manager writable by default
(mount_read_only -> false) which exposes write/delete commands (write_file,
edit_file, move_file, create_directory) without notification or gating; revert
mount_read_only to true for now and implement: (1) an upgrade/installation
prompt that surfaces catalog_revision to existing users before changing
behavior, (2) a version-check/upgrade path that compares catalog_revision and
prompts users to accept the new write capability, and (3) enforce risk gating so
ToolRisk::High operations are either blocked or require explicit user
confirmation (and surfaced differently to the model/controller) rather than
executing unconditionally. Ensure the changes reference the pengine/file-manager
config, catalog_revision, and the high-risk command names so code paths can be
located and tested.
---
Nitpick comments:
In `@src-tauri/src/modules/keywords.rs`:
- Around line 10-18: Replace the heap-allocating function all_keyword_groups
with a static slice to avoid allocating a Vec: introduce a pub const
ALL_KEYWORD_GROUPS: &'static [&'static KeywordGroup] (or pub const
ALL_KEYWORD_GROUPS: &[&KeywordGroup]) initialized with &memory::SESSION_START,
&memory::SESSION_END, &memory::DIARY_START, &memory::DIARY_END,
&ollama::keywords::THINK_ON, and update any callers of all_keyword_groups to
reference ALL_KEYWORD_GROUPS (or provide a thin fn wrapper that returns
ALL_KEYWORD_GROUPS) so callers can iterate without taking ownership.
In `@src-tauri/src/modules/mcp/client.rs`:
- Around line 105-124: Create a small constructor or helper for ToolDef that
centralizes the repeated metadata initialization and apply call: add e.g.
ToolDef::new_with_metadata(server_name: &str, name: String, description:
Option<String>, input_schema: serde_json::Value, direct_return: bool) (or
implement Default for the metadata fields) which constructs the ToolDef with
category: None and risk: ToolRisk::Low and calls tool_metadata::apply(&mut def)
before returning; then replace the inline blocks in client.rs (current anonymous
block building ToolDef), native.rs dice_named and tool_manager_named with calls
to ToolDef::new_with_metadata(...) to DRY the repeated boilerplate and ensure
apply is always invoked.
In `@src-tauri/src/modules/mcp/registry.rs`:
- Around line 445-459: push_always_on_tools currently dedupes by tool.name only
(seen.insert(tool.name.clone())), which drops tools from other providers that
share the same name; change the dedupe key to include the provider/server plus
name (e.g., combine tool.server and tool.name or use a tuple key) when inserting
into seen so each (server, name) pair is unique, and add a log-warn when
ALWAYS_ON_TOOL_NAMES matches more than one (server, name) pair for the same name
to surface ambiguity (refer to ALWAYS_ON_TOOL_NAMES, ToolDef,
push_always_on_tools, and the seen HashSet); keep existing resolve_tool
ambiguity behavior intact.
- Around line 244-257: full_tool_context currently hardcodes select_ms to 0
which leaves a stale selection time in logs after routing escalation; change
ToolRegistry::full_tool_context to accept a select_ms: u64 parameter and assign
that value to ToolContextSelection.select_ms, then update all call sites (e.g.,
where agent.rs constructs the tool context during escalation) to pass the actual
recorded selection time (the value used with state.record_tool_selection_ms) so
emit_log sees the correct select_ms; ensure the signature change is propagated
and compiled across usages.
In `@src-tauri/src/modules/mcp/tool_metadata.rs`:
- Around line 24-26: Add explicit parentheses around the boolean expression to
remove ambiguity: change the conditional that returns ("system".into(),
ToolRisk::High) to use (name == "manage_tools") || (desc.contains("uninstall")
&& desc.contains("catalog")) so the intent is explicit; update the expression in
the block where name and desc are checked (the conditional that currently reads
name == "manage_tools" || desc.contains("uninstall") &&
desc.contains("catalog")).
In `@src-tauri/src/modules/ollama/service.rs`:
- Around line 159-169: The summarize_reply_json_schema() helper belongs to the
bot layer, not the transport/service; move this function into bot/agent.rs (or a
new bot/schemas.rs) and import it where the agent builds phase-2 summaries, and
update the service module to stop owning agent-specific schemas—leave the
transport to expose only a generic ChatOptions.format: Option<serde_json::Value>
(or similar) and have code that previously referenced
summarize_reply_json_schema() point to the new location (adjust any use sites in
modules that call summarize_reply_json_schema and remove the helper from
service.rs).
In `@src-tauri/src/modules/skills/service.rs`:
- Around line 588-600: The current hack in search_clawhub that substitutes "a"
into effective_q when q is empty and opts.staff_picks is true should be made
explicit and traceable: extract the fallback into a named constant (e.g.,
STAFF_PICKS_SEED = "a") and add a TODO comment above search_clawhub referencing
an upstream issue/PR URL and describing the desired contract (allow q= with
staffPicks=true or return seeded staff picks), and if staff_picks may become the
primary UX add an implementation note (or simple improvement) to probe a small
set of seed letters (e.g., ["a","e","i"]) and merge/dedupe results instead of a
single-letter bias; ensure references to effective_q and
ClawHubSearchOptions::staff_picks remain and update any tests or comments
accordingly.
In `@src-tauri/src/modules/skills/types.rs`:
- Around line 106-112: ClawHubPluginsResponse currently uses an explicit rename
on next_cursor while other structs use #[serde(rename_all = "camelCase")];
change the struct attribute to #[serde(rename_all = "camelCase")] on
ClawHubPluginsResponse and remove the per-field rename (keep #[serde(default)]
on items and next_cursor) so new camelCase fields are handled consistently and
you don't need individual renames for next_cursor.
In `@src-tauri/src/shared/state.rs`:
- Around line 87-102: Replace the FIFO Vec-based buffers with VecDeque to avoid
O(n) remove(0) evictions: change the types of recent_tool_names and
tool_ctx_latency_ms to VecDeque<String>/VecDeque<u64> (or appropriate element
type), import std::collections::VecDeque, and update note_tools_used and
record_tool_selection_ms to use push_back to append and pop_front while len() >
RECENT_TOOLS_CAP / TOOL_CTX_LATENCY_CAP to evict oldest entries; keep existing
trimming/validation logic (trim/empty checks) but push_back the trimmed strings
and use pop_front for overflow.
In `@src/modules/skills/api/index.ts`:
- Around line 132-133: The two functions searchClawHub and searchClawHubPlugins
disagree on limit handling; compute a single resolved limit value (e.g., const
lim = filters.limit && filters.limit > 0 ? filters.limit : 30) up front and use
that resolved lim both for timeout calculation and to set the URL param (replace
the conditional url.searchParams.set("limit", String(filters.limit)) with
url.searchParams.set("limit", String(lim))). Ensure both functions use the same
resolved variable name (lim) so the timeout and request param are consistent.
- Around line 127-128: The current code always sets url.searchParams for
"highlighted" and "nonSuspicious" to true when the caller omits them (via
String(filters.highlighted !== false)), which silently applies those filters;
change this to only add those query params when the caller explicitly provides
them (e.g., check filters.highlighted !== undefined / filters.highlighted !==
null before calling url.searchParams.set for "highlighted", and do the same for
filters.nonSuspicious) to match the handling used for staffPicks/cleanOnly/tag,
and optionally add a docstring on the ClawHubSearchFilters type explaining
default behavior if keeping implicit-true was intentional.
In `@src/modules/skills/components/ClawHubBrowse.tsx`:
- Around line 234-248: The IntersectionObserver effect in ClawHubBrowse is
reconnecting on every list mutation because its dependency array includes
changing arrays like results and visiblePlugins; update the useEffect to only
depend on stable references (e.g., loadMore and clawRegistry) so it doesn't
teardown/recreate the observer on each append, keep using
listViewportRef.current and loadMoreRef.current inside the effect, and rely on
the sentinel render logic (hasMoreSkills/hasMorePlugins) to control observation
instead of including results/visiblePlugins in the deps.
- Around line 88-107: The ref assignment queryRef.current = query is performed
during render which breaks render purity; move that mutation into a useEffect
like useEffect(() => { queryRef.current = query; }, [query]) or eliminate
queryRef and read query state directly inside the callbacks (and add query to
their useCallback dependency arrays) so the ref is only updated after render and
callbacks use the correct up-to-date value; update any places referencing
queryRef to reflect this change (in the ClawHubBrowse component and related
handlers).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e97860f-6aad-42dd-b6ae-14a122034059
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.locksrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
package.jsonsrc-tauri/Cargo.tomlsrc-tauri/src/app.rssrc-tauri/src/infrastructure/http_server.rssrc-tauri/src/modules/bot/agent.rssrc-tauri/src/modules/bot/commands.rssrc-tauri/src/modules/bot/service.rssrc-tauri/src/modules/keywords.rssrc-tauri/src/modules/mcp/client.rssrc-tauri/src/modules/mcp/mod.rssrc-tauri/src/modules/mcp/native.rssrc-tauri/src/modules/mcp/registry.rssrc-tauri/src/modules/mcp/tool_metadata.rssrc-tauri/src/modules/mcp/types.rssrc-tauri/src/modules/memory/mod.rssrc-tauri/src/modules/mod.rssrc-tauri/src/modules/ollama/keywords.rssrc-tauri/src/modules/ollama/mod.rssrc-tauri/src/modules/ollama/service.rssrc-tauri/src/modules/skills/service.rssrc-tauri/src/modules/skills/types.rssrc-tauri/src/shared/keywords.rssrc-tauri/src/shared/mod.rssrc-tauri/src/shared/state.rssrc-tauri/src/shared/text.rssrc/modules/skills/api/index.tssrc/modules/skills/components/ClawHubBrowse.tsxsrc/modules/skills/components/SkillsPanel.tsxsrc/modules/skills/types.tstools/mcp-tools.jsontools/skills/weather/README.mdtools/skills/weather/mandatory.md
Comment on lines
+639
to
+665
| /// Weight recent invocations: newest names in the deque score highest. | ||
| fn recent_tool_score(tool: &ToolDef, recent: &[String]) -> usize { | ||
| let mut score = 0usize; | ||
| for (i, r) in recent.iter().enumerate() { | ||
| if tool_name_matches_recent(tool, r) { | ||
| let weight = recent.len().saturating_sub(i); | ||
| score += 8 + weight * 4; | ||
| } | ||
| } | ||
| score | ||
| } | ||
|
|
||
| fn tool_name_matches_recent(tool: &ToolDef, recent: &str) -> bool { | ||
| let r = recent.trim(); | ||
| if r.is_empty() { | ||
| return false; | ||
| } | ||
| if tool.name.eq_ignore_ascii_case(r) { | ||
| return true; | ||
| } | ||
| if let Some((_, short)) = r.rsplit_once('.') { | ||
| if tool.name.eq_ignore_ascii_case(short) { | ||
| return true; | ||
| } | ||
| } | ||
| false | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the snapshot implementation and confirm iteration order.
rg -nP --type=rust -C5 '\bfn\s+recent_tools_snapshot\s*\('
rg -nP --type=rust -C3 '\bnote_tools_used\s*\('Repository: pengine-ai/pengine
Length of output: 1724
🏁 Script executed:
sed -n '87,110p' src-tauri/src/shared/state.rsRepository: pengine-ai/pengine
Length of output: 749
Invert the weight calculation in recent_tool_score—oldest tools are currently being scored highest.
The implementation pushes new tool names to the end of recent_tool_names and removes from index 0 when capacity is exceeded, meaning index 0 is oldest and the final index is newest. However, recent_tool_score calculates weight as recent.len().saturating_sub(i), giving the highest weight to index 0 (oldest) and the lowest to the last index (newest). This contradicts the doc comment ("newest names... score highest") and inverts the intended behavior.
Fix the weight formula to i + 1 or recent.len() - len + i + 1 to correctly boost recent tools. Also add a doc comment to recent_tools_snapshot clarifying that it returns names in insertion order (oldest first).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/modules/mcp/registry.rs` around lines 639 - 665,
recent_tool_score currently gives highest weight to the oldest entry because it
uses recent.len().saturating_sub(i); change the weight calculation to give newer
indices larger weights (e.g., use weight = i + 1 or compute relative age so
later indices score higher) in the recent_tool_score function; also update/add a
doc comment on recent_tools_snapshot to state it returns names in insertion
order (oldest first) so callers know the slice is oldest-to-newest.
Comment on lines
+416
to
+493
| static SKILL_STATS_RE: OnceLock<Regex> = OnceLock::new(); | ||
| static OWNER_HANDLE_RE: OnceLock<Regex> = OnceLock::new(); | ||
| static SEMVER_RE: OnceLock<Regex> = OnceLock::new(); | ||
|
|
||
| fn skill_stats_re() -> &'static Regex { | ||
| SKILL_STATS_RE | ||
| .get_or_init(|| Regex::new(r"stats:\$R\[\d+\]=\{([^}]+)\}").expect("skill stats regex")) | ||
| } | ||
|
|
||
| fn owner_handle_re() -> &'static Regex { | ||
| OWNER_HANDLE_RE.get_or_init(|| Regex::new(r#"handle:\"([^\"]+)\""#).expect("handle regex")) | ||
| } | ||
|
|
||
| fn semver_re() -> &'static Regex { | ||
| SEMVER_RE.get_or_init(|| Regex::new(r#"version:\"(\d+\.\d+\.\d+)\""#).expect("semver regex")) | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Default)] | ||
| struct ParsedOpenclawSkillPage { | ||
| owner_handle: Option<String>, | ||
| downloads: Option<u64>, | ||
| stars: Option<u64>, | ||
| installs_current: Option<u64>, | ||
| installs_all_time: Option<u64>, | ||
| version_count: Option<u64>, | ||
| comments_count: Option<u64>, | ||
| version_semver: Option<String>, | ||
| is_highlighted: bool, | ||
| is_official: bool, | ||
| } | ||
|
|
||
| fn parse_stats_blob(blob: &str, out: &mut ParsedOpenclawSkillPage) { | ||
| for part in blob.split(',') { | ||
| let Some((k, v)) = part.split_once(':') else { | ||
| continue; | ||
| }; | ||
| let Ok(n) = v.trim().parse::<u64>() else { | ||
| continue; | ||
| }; | ||
| match k.trim() { | ||
| "downloads" => out.downloads = Some(n), | ||
| "stars" => out.stars = Some(n), | ||
| "installsCurrent" => out.installs_current = Some(n), | ||
| "installsAllTime" => out.installs_all_time = Some(n), | ||
| "versions" => out.version_count = Some(n), | ||
| "comments" => out.comments_count = Some(n), | ||
| _ => {} | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn extract_owner_before_skill(html: &str) -> Option<String> { | ||
| let idx = html.find("skill:$R")?; | ||
| let prefix = &html[..idx]; | ||
| owner_handle_re() | ||
| .captures_iter(prefix) | ||
| .last() | ||
| .and_then(|c| c.get(1)) | ||
| .map(|m| m.as_str().to_string()) | ||
| } | ||
|
|
||
| fn parse_openclaw_skill_html(html: &str) -> ParsedOpenclawSkillPage { | ||
| let mut out = ParsedOpenclawSkillPage::default(); | ||
| if let Some(cap) = skill_stats_re().captures(html) { | ||
| if let Some(m) = cap.get(1) { | ||
| parse_stats_blob(m.as_str(), &mut out); | ||
| } | ||
| } | ||
| out.owner_handle = extract_owner_before_skill(html); | ||
| if let Some(c) = semver_re().captures(html) { | ||
| if let Some(m) = c.get(1) { | ||
| out.version_semver = Some(m.as_str().to_string()); | ||
| } | ||
| } | ||
| out.is_highlighted = html.contains("highlighted:$R"); | ||
| out.is_official = html.contains("official:$R"); | ||
| out | ||
| } |
There was a problem hiding this comment.
Regex-scraped Next.js RSC payload is brittle — confirm the failure path is silent.
The three regexes (stats:\$R\[\d+\]=\{…\}, handle:"…", version:"x.y.z") and the skill:$R / highlighted:$R / official:$R string matches target an internal RSC serialization format that ClawHub can change without notice (renamed globals, pretty-printed output, schema tweaks). The good news is every field on ParsedOpenclawSkillPage is Option/bool, merge_openclaw_parsed only overwrites on Some, and fetch_openclaw_skill_html returns None on any error — so a format change degrades gracefully to "no enrichment" rather than a hard failure. Worth a comment documenting that contract so a future maintainer doesn't "fix" expect("skill stats regex") into a propagating error.
Also note parse_stats_blob splits on , / : and silently drops any entry with nested separators; acceptable for the current flat numeric payload but would miss quoted values containing commas if ClawHub ever inlines them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/modules/skills/service.rs` around lines 416 - 493, Add a clear
comment near the regex statics (SKILL_STATS_RE, OWNER_HANDLE_RE, SEMVER_RE) and
the parsing functions (skill_stats_re, owner_handle_re, semver_re,
parse_openclaw_skill_html, parse_stats_blob, extract_owner_before_skill) stating
that these regexes and string matches target an internal Next.js RSC
serialization format and are intentionally brittle: failures degrade silently
(ParsedOpenclawSkillPage fields are Option/bool, merge_openclaw_parsed only
overwrites on Some, and fetch_openclaw_skill_html returns None on error), so do
not convert the expect(...) calls into propagated errors; also note
parse_stats_blob’s simplistic split on ',' and ':' will ignore nested/quoted
commas and is acceptable for current flat numeric payloads but may miss future
quoted values.
Comment on lines
+529
to
+552
| async fn enrich_clawhub_skills_from_openclaw( | ||
| client: &reqwest::Client, | ||
| skills: &mut [ClawHubSkill], | ||
| ) { | ||
| let futs: Vec<_> = skills | ||
| .iter() | ||
| .map(|s| s.slug.clone()) | ||
| .map(|slug| { | ||
| let client = client.clone(); | ||
| async move { | ||
| let html = fetch_openclaw_skill_html(&client, &slug).await?; | ||
| let parsed = parse_openclaw_skill_html(&html); | ||
| Some((slug, parsed)) | ||
| } | ||
| }) | ||
| .collect(); | ||
| let pairs: HashMap<String, ParsedOpenclawSkillPage> = | ||
| join_all(futs).await.into_iter().flatten().collect(); | ||
| for sk in skills.iter_mut() { | ||
| if let Some(p) = pairs.get(&sk.slug) { | ||
| merge_openclaw_parsed(sk, p); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the futures::stream::StreamExt::buffer_unordered signature and is it the idiomatic way to bound concurrency for async HTTP fan-out in Rust?
💡 Result:
The signature of futures::stream::StreamExt::buffer_unordered is: rust fn buffer_unordered(self, n: usize) -> BufferUnordered where Self::Item: Future, Self: Sized, This method creates an adaptor for buffering a list of pending futures from the stream, executing up to n futures concurrently in an unordered manner (results yielded as they complete). Yes, it is the idiomatic way to bound concurrency for async HTTP fan-out in Rust. It is commonly recommended in Rust community discussions and tutorials for scenarios like parallel HTTP requests where order doesn't matter, as it naturally limits the number of in-flight futures without spawning unnecessary tasks or using semaphores. For ordered results, buffered is the alternative, but buffer_unordered is preferred for lower latency in fan-out patterns.
Citations:
- 1: https://docs.rs/futures/latest/futures/stream/trait.StreamExt.html
- 2: https://docs.rs/futures/latest/futures/stream/struct.BufferUnordered.html
- 3: https://docs.rs/futures/0.3.19/futures/stream/struct.BufferUnordered.html
- 4: https://gendignoux.com/blog/2021/04/01/rust-async-streams-futures-part1.html
- 5: https://medium.com/@jaderd/rust-bounded-concurrency-without-semaphores-368ff683ebdc
- 6: https://users.rust-lang.org/t/idiomatic-way-of-parallel-execution-of-unordered-futures/80474
Cap concurrency on enrichment HTTP fetches to clawhub.ai.
enrich_clawhub_skills_from_openclaw spawns one concurrent future per skill via join_all, and search_clawhub clamps limit to 1..=500 (line 605) with opts.enrich defaulting to true. A single user search with limit=500 therefore issues up to 500 concurrent HTTPS GETs to clawhub.ai from one client, risking:
- ClawHub rate-limiting / IP blocks that break enrichment for all users
- Local socket / file-descriptor exhaustion on constrained machines
- Simultaneous buffering of ~500 HTML-page sizes in memory
Use futures::stream::iter(...).buffer_unordered(8) to cap in-flight requests:
Fix: bounded concurrency
- let futs: Vec<_> = skills
- .iter()
- .map(|s| s.slug.clone())
- .map(|slug| {
- let client = client.clone();
- async move {
- let html = fetch_openclaw_skill_html(&client, &slug).await?;
- let parsed = parse_openclaw_skill_html(&html);
- Some((slug, parsed))
- }
- })
- .collect();
- let pairs: HashMap<String, ParsedOpenclawSkillPage> =
- join_all(futs).await.into_iter().flatten().collect();
+ use futures::stream::{self, StreamExt};
+ const ENRICH_CONCURRENCY: usize = 8;
+ let slugs: Vec<String> = skills.iter().map(|s| s.slug.clone()).collect();
+ let pairs: HashMap<String, ParsedOpenclawSkillPage> = stream::iter(slugs)
+ .map(|slug| {
+ let client = client.clone();
+ async move {
+ let html = fetch_openclaw_skill_html(&client, &slug).await?;
+ Some((slug, parse_openclaw_skill_html(&html)))
+ }
+ })
+ .buffer_unordered(ENRICH_CONCURRENCY)
+ .filter_map(|x| async move { x })
+ .collect()
+ .await;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/modules/skills/service.rs` around lines 529 - 552, The current
enrich_clawhub_skills_from_openclaw spawns one future per skill and uses
join_all, causing unbounded concurrency; replace that logic with a bounded
stream using futures::stream::iter(...) and .map(|slug| { let client =
client.clone(); async move { let html = fetch_openclaw_skill_html(&client,
&slug).await?; let parsed = parse_openclaw_skill_html(&html); Some((slug,
parsed)) } }).buffer_unordered(8).collect::<Vec<_>>().await (or similar) so at
most ~8 HTTP fetches are in-flight; then flatten/collect the results into the
HashMap<String, ParsedOpenclawSkillPage> and proceed to merge_openclaw_parsed as
before. Ensure you reference enrich_clawhub_skills_from_openclaw,
fetch_openclaw_skill_html, parse_openclaw_skill_html and merge_openclaw_parsed
when making the change.
Comment on lines
+109
to
+175
| const runSkillsSearch = useCallback( | ||
| async (q: string, limit: number, loadMore = false) => { | ||
| const trimmed = q.trim(); | ||
| if (!trimmed) return; | ||
| if (loadMore) setLoadingMore(true); | ||
| else setBrowseLoading(true); | ||
| setBrowseError(null); | ||
| setPluginResults(null); | ||
| const result = await searchClawHub(trimmed, { | ||
| cleanOnly: filterCleanOnly, | ||
| sort: clawSort, | ||
| limit, | ||
| }); | ||
| if (loadMore) setLoadingMore(false); | ||
| else setBrowseLoading(false); | ||
| if (result.results) { | ||
| if (loadMore) { | ||
| setResults((prev) => { | ||
| if (!prev?.length) return result.results!; | ||
| const next = result.results!; | ||
| if (next.length <= prev.length) return prev; | ||
| return [...prev, ...next.slice(prev.length)]; | ||
| }); | ||
| } else { | ||
| setResults(result.results); | ||
| } | ||
| setHasMoreSkills(result.results.length >= limit); | ||
| } else { | ||
| setBrowseError(result.error ?? "ClawHub is unreachable"); | ||
| setHasMoreSkills(false); | ||
| } | ||
| }, | ||
| [filterCleanOnly, clawSort], | ||
| ); | ||
|
|
||
| const runPluginsSearch = useCallback(async (q: string, loadMore = false) => { | ||
| if (loadMore) { | ||
| if (!pluginCursorRef.current) return; | ||
| setLoadingMore(true); | ||
| } else { | ||
| pluginCursorRef.current = null; | ||
| setBrowseLoading(true); | ||
| } | ||
| setBrowseError(null); | ||
| setResults(null); | ||
| const cursor = loadMore ? (pluginCursorRef.current ?? undefined) : undefined; | ||
| const result = await searchClawHubPlugins(q.trim(), { | ||
| limit: SEARCH_PAGE_SIZE, | ||
| cursor, | ||
| timeoutMs: Math.min(120_000, 22_000 + SEARCH_PAGE_SIZE * 500), | ||
| }); | ||
| if (loadMore) setLoadingMore(false); | ||
| else setBrowseLoading(false); | ||
| if (result.items) { | ||
| const nextCur = result.nextCursor?.trim() || null; | ||
| pluginCursorRef.current = nextCur; | ||
| if (loadMore) { | ||
| setPluginResults((prev) => [...(prev ?? []), ...result.items!]); | ||
| } else { | ||
| setPluginResults(result.items); | ||
| } | ||
| setHasMorePlugins(nextCur != null && nextCur.length > 0); | ||
| } else { | ||
| setBrowseError(result.error ?? "ClawHub plugins unreachable"); | ||
| setHasMorePlugins(false); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Stale-response race when filters/tabs change during in-flight searches.
Neither runSkillsSearch nor runPluginsSearch guards against concurrency. A user rapidly toggling filterCleanOnly, changing clawSort, or switching tabs while a request is in flight will trigger a second fetch before the first resolves; whichever completes last overwrites the state. Results can end up mismatched against the current filter/tab selection, and setPluginResults(null) on line 153 from a skills-tab follow-up can even blow away a just-loaded plugins list.
Consider either an AbortController per run (canceling on dep change) or a generation counter checked before each setResults/setPluginResults call.
🛠️ Sketch: generation-counter guard
const pluginCursorRef = useRef<string | null>(null);
+ const skillsReqIdRef = useRef(0);
+ const pluginsReqIdRef = useRef(0);
const runSkillsSearch = useCallback(
async (q: string, limit: number, loadMore = false) => {
const trimmed = q.trim();
if (!trimmed) return;
+ const reqId = ++skillsReqIdRef.current;
if (loadMore) setLoadingMore(true);
else setBrowseLoading(true);
...
const result = await searchClawHub(trimmed, { ... });
+ if (reqId !== skillsReqIdRef.current) return; // stale
...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/skills/components/ClawHubBrowse.tsx` around lines 109 - 175, Both
runSkillsSearch and runPluginsSearch can produce stale overwrites when toggling
filterCleanOnly, clawSort, or switching tabs while a request is in flight; add a
per-search generation guard or AbortController so only the latest response
updates state. Implement by keeping a mutable ref (e.g., searchGenRef) that you
increment before each new invocation of runSkillsSearch/runPluginsSearch (or
create/replace an AbortController and pass its signal), capture the current
token inside the async call, and before calling setResults, setPluginResults,
setHasMoreSkills, setHasMorePlugins, setBrowseError, or changing
pluginCursorRef.current verify the token matches the latest ref (or that the
AbortSignal is not aborted); cancel/ignore older responses so in-flight
completions cannot clobber newer UI state.
Comment on lines
+192
to
+199
| const visiblePlugins = useMemo(() => { | ||
| if (!pluginResults) return null; | ||
| const t = pluginTagFilter.trim().toLowerCase(); | ||
| if (!t) return pluginResults; | ||
| return pluginResults.filter((p) => | ||
| p.capabilityTags.some((tag) => tag.toLowerCase().includes(t)), | ||
| ); | ||
| }, [pluginResults, pluginTagFilter]); |
There was a problem hiding this comment.
Plugin tag filter only sees already-loaded pages.
visiblePlugins filters the local pluginResults array, so if the user types a tag that only matches plugins on later pages, nothing appears and they may assume no matches exist — the UI copy on lines 385-387 correctly notes this is client-side, but the infinite-scroll sentinel only renders inside the filtered list. If the filter hides all loaded items, the sentinel disappears too and auto-load stops. Consider either rendering the sentinel even when visiblePlugins is empty (while hasMorePlugins is true) or showing a "Load more to search further" affordance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/skills/components/ClawHubBrowse.tsx` around lines 192 - 199, The
client-side tag filter (visiblePlugins computed from pluginResults and
pluginTagFilter) can hide all loaded items so the infinite-scroll sentinel
disappears and further pages never auto-load; update the rendering logic that
places the sentinel/infinite-scroll trigger so it renders when hasMorePlugins is
true regardless of visiblePlugins.length (or show a "Load more to search
further" affordance when visiblePlugins is empty and hasMorePlugins is true),
and ensure the sentinel's container references visiblePlugins, pluginResults,
pluginTagFilter, and hasMorePlugins so users can continue loading additional
pages to search beyond the currently-loaded results.
| ], | ||
| "container_read_only_rootfs": false, | ||
| "mount_read_only": true, | ||
| "mount_read_only": false, |
There was a problem hiding this comment.
❓ Verification inconclusive
MCP filesystem server best practices read-only mount default
For the Model Context Protocol (MCP) Filesystem Server (@modelcontextprotocol/server-filesystem), the best practice for read-only mounts is to use Docker's ro (read-only) flag when mounting directories. This enforces read-only access at the OS level, preventing write operations even if the AI agent requests them. Key best practices: - Mount project directories read-write where needed, e.g., --mount type=bind,src=/path/to/project,dst=/projects/project - Mount documentation or reference directories read-only, e.g., --mount type=bind,src=/path/to/docs,dst=/projects/docs,ro - All mounts must target /projects/ prefix in the container. - Use the narrowest possible directory paths (e.g., project root, not home directory) to limit access. - Verify allowed directories with the list_allowed_directories tool. - The server itself marks read-only tools (like read_file, list_directory) via MCP ToolAnnotations, but OS-level ro provides strongest enforcement. - No built-in per-directory read-only flag in the Node.js server CLI; Docker ro is the recommended method. Example Docker config for Claude Desktop or VS Code mcp.json: { "mcpServers": { "filesystem": { "command": "docker", "args": [ "run", "-i", "--rm", "--mount", "type=bind,src=${workspaceFolder},dst=/projects/workspace", "--mount", "type=bind,src=/path/to/docs,dst=/projects/docs,ro", "mcp/filesystem", "/projects/workspace", "/projects/docs" ] } } } There is no "default" read-only mount; all directories are read-write unless explicitly mounted ro. For agents that only need reads, mount everything ro. Dedicated read-only server variants exist but official uses Docker ro. Official GitHub: https://github.com/modelcontextprotocol/servers/tree/main/src/filesystem
Citations:
- 1: https://markaicode.com/mcp-filesystem-server-safe-read-write/
- 2: https://github.com/modelcontextprotocol/servers/blob/main/src/filesystem/README.md
- 3: https://playbooks.com/mcp/modelcontextprotocol-filesystem
- 4: https://mcpindex.net/en/mcpserver/modelcontextprotocol-server-filesystem
- 5: https://mcprepository.com/modelcontextprotocol/filesystem
- 6: https://github.com/modelcontextprotocol/servers/blob/e6b0b0f5/src/filesystem/README.md
- 7: https://www.npmjs.com/package/@modelcontextprotocol/server-filesystem
- 8: https://github.com/modelcontextprotocol/servers/tree/main/src/filesystem
Script executed:
# First, let's check the structure and find the tools/mcp-tools.json file
find . -name "mcp-tools.json" -o -name "*mcp-tools*" | head -20Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
./doc/custom-mcp-tools.md
./tools/mcp-tools.json
Script executed:
# Look at the tools/mcp-tools.json file around line 25
wc -l tools/mcp-tools.jsonRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
257 tools/mcp-tools.json
Script executed:
# Read the file to understand its structure
cat -n tools/mcp-tools.json | head -60Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
1 {
2 "schema_version": 1,
3 "generated_at": "2026-04-14T00:00:00Z",
4 "catalog_revision": 7,
5 "valid_until": "2026-05-14T00:00:00Z",
6 "minimum_pengine_version": "0.5.0",
7 "tools": [
8 {
9 "id": "pengine/file-manager",
10 "name": "File Manager",
11 "description": "Filesystem MCP in a container. Add folders in MCP Tools; each mounts at /app/<folder-name>. Install works before any folder is set.",
12 "image": "ghcr.io/pengine-ai/pengine-file-manager",
13 "current": "0.1.0",
14 "versions": [
15 {
16 "version": "0.1.0",
17 "digest": "sha256:placeholder",
18 "released_at": "2026-04-12T00:00:00Z",
19 "yanked": false,
20 "revoked": false,
21 "security": false
22 }
23 ],
24 "container_read_only_rootfs": false,
25 "mount_read_only": false,
26 "mount_workspace": true,
27 "append_workspace_roots": true,
28 "direct_return": true,
29 "upstream_mcp_npm": {
30 "package": "@modelcontextprotocol/server-filesystem",
31 "version": "2026.1.14"
32 },
33 "mcp_server_cmd": [],
34 "commands": [
35 { "name": "read_text_file", "description": "Read a file as UTF-8 text; optional head/tail line limits" },
36 { "name": "read_media_file", "description": "Read image or audio as base64 with MIME type" },
37 { "name": "read_multiple_files", "description": "Read several files in one call" },
38 { "name": "write_file", "description": "Create or overwrite a file" },
39 { "name": "edit_file", "description": "Pattern-based selective edits with optional dry run" },
40 { "name": "create_directory", "description": "Create a directory (and parents)" },
41 { "name": "list_directory", "description": "List entries with [FILE]/[DIR] prefixes" },
42 { "name": "list_directory_with_sizes", "description": "List directory with sizes and optional sort" },
43 { "name": "move_file", "description": "Move or rename a file or directory" },
44 { "name": "search_files", "description": "Recursive glob search under a path" },
45 { "name": "directory_tree", "description": "Recursive JSON tree of directory contents" },
46 { "name": "get_file_info", "description": "Metadata: size, times, type, permissions" },
47 { "name": "list_allowed_directories", "description": "List MCP roots currently allowed" }
48 ],
49 "limits": {
50 "cpus": "0.5",
51 "memory": "256m",
52 "timeout_secs": 30
53 }
54 },
55 {
56 "id": "pengine/fetch",
57 "name": "Fetch",
58 "description": "Official MCP fetch server: fetches URLs and returns markdown-friendly content. By default the server respects robots.txt. To opt in for this install, set catalog field ignore_robots_txt to true (appends --ignore-robots-txt) or add that flag under mcp_server_cmd in advanced config — only if you accept bypassing robots for all fetched hosts. The robots_ignore_allowlist field is reserved for future per-host policy and is currently informational.",
59 "image": "ghcr.io/pengine-ai/pengine-fetch",
60 "current": "0.1.0",
Script executed:
# Search for mount_read_only in the file
rg "mount_read_only" -B 5 -A 5Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
tools/mcp-tools.json- "revoked": false,
tools/mcp-tools.json- "security": false
tools/mcp-tools.json- }
tools/mcp-tools.json- ],
tools/mcp-tools.json- "container_read_only_rootfs": false,
tools/mcp-tools.json: "mount_read_only": false,
tools/mcp-tools.json- "mount_workspace": true,
tools/mcp-tools.json- "append_workspace_roots": true,
tools/mcp-tools.json- "direct_return": true,
tools/mcp-tools.json- "upstream_mcp_npm": {
tools/mcp-tools.json- "package": "@modelcontextprotocol/server-filesystem",
--
tools/mcp-tools.json- "revoked": false,
tools/mcp-tools.json- "security": false
tools/mcp-tools.json- }
tools/mcp-tools.json- ],
tools/mcp-tools.json- "container_read_only_rootfs": true,
tools/mcp-tools.json: "mount_read_only": true,
tools/mcp-tools.json- "mount_workspace": false,
tools/mcp-tools.json- "append_workspace_roots": false,
tools/mcp-tools.json- "direct_return": true,
tools/mcp-tools.json- "network_isolated": false,
tools/mcp-tools.json- "upstream_mcp_pypi": {
--
tools/mcp-tools.json- "revoked": false,
tools/mcp-tools.json- "security": false
tools/mcp-tools.json- }
tools/mcp-tools.json- ],
tools/mcp-tools.json- "container_read_only_rootfs": false,
tools/mcp-tools.json: "mount_read_only": false,
tools/mcp-tools.json- "mount_workspace": true,
tools/mcp-tools.json- "append_workspace_roots": false,
tools/mcp-tools.json- "direct_return": true,
tools/mcp-tools.json- "upstream_mcp_pypi": {
tools/mcp-tools.json- "package": "mcp-server-git",
--
tools/mcp-tools.json- "revoked": false,
tools/mcp-tools.json- "security": false
tools/mcp-tools.json- }
tools/mcp-tools.json- ],
tools/mcp-tools.json- "container_read_only_rootfs": true,
tools/mcp-tools.json: "mount_read_only": true,
tools/mcp-tools.json- "mount_workspace": false,
tools/mcp-tools.json- "append_workspace_roots": false,
tools/mcp-tools.json- "direct_return": false,
tools/mcp-tools.json- "upstream_mcp_npm": {
tools/mcp-tools.json- "package": "@modelcontextprotocol/server-sequential-thinking",
--
tools/mcp-tools.json- "revoked": false,
tools/mcp-tools.json- "security": false
tools/mcp-tools.json- }
tools/mcp-tools.json- ],
tools/mcp-tools.json- "container_read_only_rootfs": true,
tools/mcp-tools.json: "mount_read_only": true,
tools/mcp-tools.json- "mount_workspace": false,
tools/mcp-tools.json- "append_workspace_roots": false,
tools/mcp-tools.json- "direct_return": true,
tools/mcp-tools.json- "upstream_mcp_pypi": {
tools/mcp-tools.json- "package": "mcp-server-time",
--
tools/mcp-tools.json- "revoked": false,
tools/mcp-tools.json- "security": false
tools/mcp-tools.json- }
tools/mcp-tools.json- ],
tools/mcp-tools.json- "container_read_only_rootfs": false,
tools/mcp-tools.json: "mount_read_only": true,
tools/mcp-tools.json- "mount_workspace": false,
tools/mcp-tools.json- "append_workspace_roots": false,
tools/mcp-tools.json- "direct_return": false,
tools/mcp-tools.json- "upstream_mcp_npm": {
tools/mcp-tools.json- "package": "@modelcontextprotocol/server-memory",
--
doc/custom-mcp-tools.md- "key": "my-mcp",
doc/custom-mcp-tools.md- "name": "My MCP container",
doc/custom-mcp-tools.md- "image": "ghcr.io/your-org/your-mcp-image:latest",
doc/custom-mcp-tools.md- "mcp_server_cmd": [],
doc/custom-mcp-tools.md- "mount_workspace": true,
doc/custom-mcp-tools.md: "mount_read_only": true,
doc/custom-mcp-tools.md- "append_workspace_roots": true,
doc/custom-mcp-tools.md- "direct_return": false
doc/custom-mcp-tools.md-}
doc/custom-mcp-tools.md-```
doc/custom-mcp-tools.md-
--
doc/custom-mcp-tools.md-| `key` | Stable id; server name becomes `te_custom_<key>`. Must be unique. |
doc/custom-mcp-tools.md-| `name` | Human-readable label (stored in config; not the MCP server key). |
doc/custom-mcp-tools.md-| `image` | Full image reference to `pull` / `run`. |
doc/custom-mcp-tools.md-| `mcp_server_cmd` | Extra arguments **after** the image (e.g. if the image needs a subcommand). Often `[]`. |
doc/custom-mcp-tools.md-| `mount_workspace` | If true, bind-mount **`workspace_roots`** from `mcp.json` into the container (same `/app/<basename>` layout as File Manager). |
doc/custom-mcp-tools.md:| `mount_read_only` | Use `:ro` on those binds when `mount_workspace` is true. |
doc/custom-mcp-tools.md-| `append_workspace_roots` | If true, append container paths (`/app/...`) to the argv (for servers that expect roots as trailing args). |
doc/custom-mcp-tools.md-| `direct_return` | If true, tool results go straight to the user without a second model pass. |
doc/custom-mcp-tools.md-
doc/custom-mcp-tools.md-**Bundled catalog (`tools/mcp-tools.json`):** the Fetch entry uses `ignore_robots_txt` (default `false`) so robots.txt is honored unless you opt in. Setting it to `true` appends `--ignore-robots-txt` for that tool. The `robots_ignore_allowlist` field is reserved for future per-host behavior and is informational today.
doc/custom-mcp-tools.md-
--
src-tauri/src/modules/tool_engine/service.rs- } else {
src-tauri/src/modules/tool_engine/service.rs- Vec::new()
src-tauri/src/modules/tool_engine/service.rs- };
src-tauri/src/modules/tool_engine/service.rs-
src-tauri/src/modules/tool_engine/service.rs- if entry.mount_workspace && !bind_pairs.is_empty() {
src-tauri/src/modules/tool_engine/service.rs: let suffix = if entry.mount_read_only { "ro" } else { "rw" };
src-tauri/src/modules/tool_engine/service.rs- args.extend(
src-tauri/src/modules/tool_engine/service.rs- bind_pairs
src-tauri/src/modules/tool_engine/service.rs- .iter()
src-tauri/src/modules/tool_engine/service.rs- .map(|(host, cpath)| format!("-v={host}:{cpath}:{suffix}")),
src-tauri/src/modules/tool_engine/service.rs- );
--
src-tauri/src/modules/tool_engine/service.rs- } else {
src-tauri/src/modules/tool_engine/service.rs- Vec::new()
src-tauri/src/modules/tool_engine/service.rs- };
src-tauri/src/modules/tool_engine/service.rs-
src-tauri/src/modules/tool_engine/service.rs- if entry.mount_workspace && !bind_pairs.is_empty() {
src-tauri/src/modules/tool_engine/service.rs: let suffix = if entry.mount_read_only { "ro" } else { "rw" };
src-tauri/src/modules/tool_engine/service.rs- args.extend(
src-tauri/src/modules/tool_engine/service.rs- bind_pairs
src-tauri/src/modules/tool_engine/service.rs- .iter()
src-tauri/src/modules/tool_engine/service.rs- .map(|(host, cpath)| format!("-v={host}:{cpath}:{suffix}")),
src-tauri/src/modules/tool_engine/service.rs- );
--
src-tauri/src/modules/tool_engine/types.rs- /// When true, add `--read-only` to the container run (rootfs).
src-tauri/src/modules/tool_engine/types.rs- #[serde(default = "default_true")]
src-tauri/src/modules/tool_engine/types.rs- pub container_read_only_rootfs: bool,
src-tauri/src/modules/tool_engine/types.rs- /// When true, use `:ro` on volume binds.
src-tauri/src/modules/tool_engine/types.rs- #[serde(default = "default_true")]
src-tauri/src/modules/tool_engine/types.rs: pub mount_read_only: bool,
src-tauri/src/modules/tool_engine/types.rs- /// When true, `podman|docker run` bind-mounts each allow-list folder under `/app/<basename>`.
src-tauri/src/modules/tool_engine/types.rs- #[serde(default)]
src-tauri/src/modules/tool_engine/types.rs- pub mount_workspace: bool,
src-tauri/src/modules/tool_engine/types.rs- /// When true, append allowed container roots after `image` + `mcp_server_cmd` (for MCP servers
src-tauri/src/modules/tool_engine/types.rs- /// like `@modelcontextprotocol/server-filesystem` that take roots as argv). Requires `mount_workspace`.
--
src-tauri/src/modules/mcp/types.rs- /// Bind-mount workspace folders into the container.
src-tauri/src/modules/mcp/types.rs- #[serde(default)]
src-tauri/src/modules/mcp/types.rs- pub mount_workspace: bool,
src-tauri/src/modules/mcp/types.rs- /// Use `:ro` on bind mounts (ignored when mount_workspace is false).
src-tauri/src/modules/mcp/types.rs- #[serde(default = "super_default_true")]
src-tauri/src/modules/mcp/types.rs: pub mount_read_only: bool,
src-tauri/src/modules/mcp/types.rs- /// Append container mount paths as argv (for MCP servers that take roots as args).
src-tauri/src/modules/mcp/types.rs- #[serde(default)]
src-tauri/src/modules/mcp/types.rs- pub append_workspace_roots: bool,
src-tauri/src/modules/mcp/types.rs- /// Return tool results directly to user without model summarisation.
src-tauri/src/modules/mcp/types.rs- #[serde(default)]
--
src-tauri/src/infrastructure/http_server.rs- serde_json::json!({
src-tauri/src/infrastructure/http_server.rs- "key": t.key,
src-tauri/src/infrastructure/http_server.rs- "name": t.name,
src-tauri/src/infrastructure/http_server.rs- "image": t.image,
src-tauri/src/infrastructure/http_server.rs- "mount_workspace": t.mount_workspace,
src-tauri/src/infrastructure/http_server.rs: "mount_read_only": t.mount_read_only,
src-tauri/src/infrastructure/http_server.rs- "append_workspace_roots": t.append_workspace_roots,
src-tauri/src/infrastructure/http_server.rs- "direct_return": t.direct_return,
src-tauri/src/infrastructure/http_server.rs- })
src-tauri/src/infrastructure/http_server.rs- })
src-tauri/src/infrastructure/http_server.rs- .collect();
--
src-tauri/src/infrastructure/http_server.rs- #[serde(default)]
src-tauri/src/infrastructure/http_server.rs- mcp_server_cmd: Vec<String>,
src-tauri/src/infrastructure/http_server.rs- #[serde(default)]
src-tauri/src/infrastructure/http_server.rs- mount_workspace: bool,
src-tauri/src/infrastructure/http_server.rs- #[serde(default = "default_true_serde")]
src-tauri/src/infrastructure/http_server.rs: mount_read_only: bool,
src-tauri/src/infrastructure/http_server.rs- #[serde(default)]
src-tauri/src/infrastructure/http_server.rs- append_workspace_roots: bool,
src-tauri/src/infrastructure/http_server.rs- #[serde(default)]
src-tauri/src/infrastructure/http_server.rs- direct_return: bool,
src-tauri/src/infrastructure/http_server.rs-}
--
src-tauri/src/infrastructure/http_server.rs- key: body.key.clone(),
src-tauri/src/infrastructure/http_server.rs- name: body.name,
src-tauri/src/infrastructure/http_server.rs- image: body.image,
src-tauri/src/infrastructure/http_server.rs- mcp_server_cmd: body.mcp_server_cmd,
src-tauri/src/infrastructure/http_server.rs- mount_workspace: body.mount_workspace,
src-tauri/src/infrastructure/http_server.rs: mount_read_only: body.mount_read_only,
src-tauri/src/infrastructure/http_server.rs- append_workspace_roots: body.append_workspace_roots,
src-tauri/src/infrastructure/http_server.rs- direct_return: body.direct_return,
src-tauri/src/infrastructure/http_server.rs- };
src-tauri/src/infrastructure/http_server.rs-
src-tauri/src/infrastructure/http_server.rs- {
Script executed:
# Check git history for mcp-tools.json to see the actual change
git log --oneline -20 tools/mcp-tools.jsonRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
73cd07a feat: enhance tool selection and memory handling during model turns
Script executed:
# Check the specific commit to see if mount_read_only changed from true to false
git show HEAD:tools/mcp-tools.json | grep -A 2 -B 2 "mount_read_only" | head -20Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
],
"container_read_only_rootfs": false,
"mount_read_only": false,
"mount_workspace": true,
"append_workspace_roots": true,
--
],
"container_read_only_rootfs": true,
"mount_read_only": true,
"mount_workspace": false,
"append_workspace_roots": false,
--
],
"container_read_only_rootfs": false,
"mount_read_only": false,
"mount_workspace": true,
"append_workspace_roots": false,
--
],
"container_read_only_rootfs": true,
Script executed:
# Check previous version to see if it was true before
git log --all -p tools/mcp-tools.json | grep -A 3 -B 3 '"mount_read_only".*true' | head -40Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
+ }
+ ],
+ "container_read_only_rootfs": true,
+ "mount_read_only": true,
+ "mount_workspace": false,
+ "append_workspace_roots": false,
+ "direct_return": true,
--
+ }
+ ],
+ "container_read_only_rootfs": true,
+ "mount_read_only": true,
+ "mount_workspace": false,
+ "append_workspace_roots": false,
+ "direct_return": false,
--
+ }
+ ],
+ "container_read_only_rootfs": true,
+ "mount_read_only": true,
+ "mount_workspace": false,
+ "append_workspace_roots": false,
+ "direct_return": true,
--
+ }
+ ],
+ "container_read_only_rootfs": false,
+ "mount_read_only": true,
+ "mount_workspace": false,
+ "append_workspace_roots": false,
+ "direct_return": false,
--
+ }
+ ],
+ "container_read_only_rootfs": true,
+ "mount_read_only": true,
+ "mount_workspace": false,
+ "append_workspace_roots": false,
+ "direct_return": true,
--
Script executed:
# Search for risk or category fields in the tools.json file
grep -E '"risk"|"category"' tools/mcp-tools.jsonRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 1
Script executed:
# Check if there are risk/category fields defined anywhere in the codebase
rg '"risk"' -B 2 -A 2Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 1
Script executed:
# Look for upgrade or notification handling logic
find . -name "*.rs" -o -name "*.ts" -o -name "*.js" | head -50 | xargs grep -l "upgrade\|catalog_revision" 2>/dev/nullRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
./src-tauri/src/modules/tool_engine/service.rs
./src-tauri/src/modules/tool_engine/types.rs
Script executed:
# Get the full diff of the commit that changed the file
git show 73cd07a --statRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
commit 73cd07a8bd6535224310a7d1a74ac18b39e3bd96
Author: MaximEdogawa <leo@librem.one>
Date: Fri Apr 17 17:58:28 2026 +0200
feat: enhance tool selection and memory handling during model turns
- Updated the tool selection process to incorporate chat session recording status, allowing for more context-aware tool routing.
- Refactored the `select_tools_for_turn` function to include a new parameter for chat session recording, improving decision-making based on user interactions.
- Enhanced logging to include chat session recording information, aiding in performance monitoring and debugging.
- Introduced new utility functions to determine the relevance of memory tools based on user messages and recent tool usage, optimizing memory API calls during interactions.
.github/ISSUE_TEMPLATE/bug_report.md | 40 +
.github/ISSUE_TEMPLATE/feature-template.md | 44 +
.github/ISSUE_TEMPLATE/technical_debt.md | 32 +
.github/scripts/tools-publish/detect-matrix.sh | 52 +
.../tools-publish/merge-multiarch-manifest.sh | 37 +
.../scripts/tools-publish/read-tool-manifest.sh | 49 +
.github/scripts/tools-publish/smoke-test-mcp.sh | 23 +
.../scripts/tools-publish/write-publish-summary.sh | 24 +
.github/workflows/tools-publish.yml | 126 +
.gitignore | 29 +
.husky/pre-commit | 5 +
.nvmrc | 1 +
.prettierrc | 10 +
.vscode/extensions.json | 3 +
LICENSE | 21 +
README.md | 133 +
bun.lock | 730 +++
doc/custom-mcp-tools.md | 203 +
doc/design/README.md | 228 +
doc/design/mcp.md | 116 +
doc/skills.md | 144 +
doc/tool-engine/manual-publish.md | 138 +
e2e/setup-dashboard.spec.ts | 239 +
eslint.config.ts | 65 +
index.html | 14 +
package-lock.json | 3326 ++++++++++
package.json | 64 +
playwright.config.ts | 26 +
public/favicon-32.png | Bin 0 -> 1882 bytes
public/pengine-logo-128.png | Bin 0 -> 15360 bytes
public/pengine-logo-64.png | Bin 0 -> 5387 bytes
rustfmt.toml | 7 +
scripts/generate-logo-assets.sh | 38 +
src-tauri/.clippy.toml | 2 +
src-tauri/.gitignore | 8 +
src-tauri/Cargo.lock | 6779 ++++++++++++++++++++
src-tauri/Cargo.toml | 43 +
src-tauri/build.rs | 3 +
src-tauri/capabilities/default.json | 14 +
src-tauri/icons/128x128.png | Bin 0 -> 15921 bytes
src-tauri/icons/128x128@2x.png | Bin 0 -> 44459 bytes
src-tauri/icons/32x32.png | Bin 0 -> 2071 bytes
src-tauri/icons/64x64.png | Bin 0 -> 5753 bytes
src-tauri/icons/StoreLogo.png | Bin 0 -> 3965 bytes
.../android/mipmap-anydpi-v26/ic_launcher.xml | 5 +
.../icons/android/mipmap-hdpi/ic_launcher.png | Bin 0 -> 3272 bytes
.../android/mipmap-hdpi/ic_launcher_foreground.png | Bin 0 -> 22030 bytes
.../android/mipmap-hdpi/ic_launcher_round.png | Bin 0 -> 3812 bytes
.../icons/android/mipmap-mdpi/ic_launcher.png | Bin 0 -> 3140 bytes
.../android/mipmap-mdpi/ic_launcher_foreground.png | Bin 0 -> 12381 bytes
.../android/mipmap-mdpi/ic_launcher_round.png | Bin 0 -> 3635 bytes
.../icons/android/mipmap-xhdpi/ic_launcher.png | Bin 0 -> 8866 bytes
.../mipmap-xhdpi/ic_launcher_foreground.png | Bin 0 -> 33993 bytes
.../android/mipmap-xhdpi/ic_launcher_round.png | Bin 0 -> 10248 bytes
.../icons/android/mipmap-xxhdpi/ic_launcher.png | Bin 0 -> 15974 bytes
.../mipmap-xxhdpi/ic_launcher_foreground.png | Bin 0 -> 65203 bytes
.../android/mipmap-xxhdpi/ic_launcher_round.png | Bin 0 -> 18548 bytes
.../icons/android/mipmap-xxxhdpi/ic_launcher.png | Bin 0 -> 24496 bytes
.../mipmap-xxxhdpi/ic_launcher_foreground.png | Bin 0 -> 109051 bytes
.../android/mipmap-xxxhdpi/ic_launcher_round.png | Bin 0 -> 28342 bytes
.../android/values/ic_launcher_background.xml | 4 +
src-tauri/icons/icon.icns | Bin 0 -> 1078287 bytes
src-tauri/icons/icon.ico | Bin 0 -> 59071 bytes
src-tauri/icons/icon.png | Bin 0 -> 151868 bytes
src-tauri/icons/ios/AppIcon-20x20@1x.png | Bin 0 -> 985 bytes
src-tauri/icons/ios/AppIcon-20x20@2x-1.png | Bin 0 -> 2819 bytes
src-tauri/icons/ios/AppIcon-20x20@2x.png | Bin 0 -> 2819 bytes
src-tauri/icons/ios/AppIcon-20x20@3x.png | Bin 0 -> 5095 bytes
src-tauri/icons/ios/AppIcon-29x29@1x.png | Bin 0 -> 1696 bytes
src-tauri/icons/ios/AppIcon-29x29@2x-1.png | Bin 0 -> 4902 bytes
src-tauri/icons/ios/AppIcon-29x29@2x.png | Bin 0 -> 4902 bytes
src-tauri/icons/ios/AppIcon-29x29@3x.png | Bin 0 -> 8891 bytes
src-tauri/icons/ios/AppIcon-40x40@1x.png | Bin 0 -> 2819 bytes
src-tauri/icons/ios/AppIcon-40x40@2x-1.png | Bin 0 -> 7917 bytes
src-tauri/icons/ios/AppIcon-40x40@2x.png | Bin 0 -> 7917 bytes
src-tauri/icons/ios/AppIcon-40x40@3x.png | Bin 0 -> 14164 bytes
src-tauri/icons/ios/AppIcon-512@2x.png | Bin 0 -> 618960 bytes
src-tauri/icons/ios/AppIcon-60x60@2x.png | Bin 0 -> 14164 bytes
src-tauri/icons/ios/AppIcon-60x60@3x.png | Bin 0 -> 25480 bytes
src-tauri/icons/ios/AppIcon-76x76@1x.png | Bin 0 -> 7318 bytes
src-tauri/icons/ios/AppIcon-76x76@2x.png | Bin 0 -> 19758 bytes
src-tauri/icons/ios/AppIcon-83.5x83.5@2x.png | Bin 0 -> 22703 bytes
src-tauri/mcp.example.json | 13 +
src-tauri/rustfmt.toml | 7 +
src-tauri/src/app.rs | 91 +
src-tauri/src/infrastructure/bot_lifecycle.rs | 29 +
src-tauri/src/infrastructure/executable_resolve.rs | 114 +
src-tauri/src/infrastructure/http_server.rs | 1462 +++++
src-tauri/src/infrastructure/mod.rs | 3 +
src-tauri/src/lib.rs | 8 +
src-tauri/src/main.rs | 6 +
src-tauri/src/modules/bot/agent.rs | 823 +++
src-tauri/src/modules/bot/commands.rs | 58 +
src-tauri/src/modules/bot/mod.rs | 4 +
src-tauri/src/modules/bot/repository.rs | 24 +
src-tauri/src/modules/bot/service.rs | 198 +
src-tauri/src/modules/keywords.rs | 47 +
src-tauri/src/modules/mcp/client.rs | 127 +
src-tauri/src/modules/mcp/mod.rs | 8 +
src-tauri/src/modules/mcp/native.rs | 362 ++
src-tauri/src/modules/mcp/protocol.rs | 74 +
src-tauri/src/modules/mcp/registry.rs | 1027 +++
src-tauri/src/modules/mcp/service.rs | 415 ++
src-tauri/src/modules/mcp/tool_metadata.rs | 51 +
src-tauri/src/modules/mcp/transport.rs | 177 +
src-tauri/src/modules/mcp/types.rs | 102 +
src-tauri/src/modules/memory/mod.rs | 376 ++
src-tauri/src/modules/mod.rs | 7 +
src-tauri/src/modules/ollama/constants.rs | 3 +
src-tauri/src/modules/ollama/keywords.rs | 85 +
src-tauri/src/modules/ollama/mod.rs | 3 +
src-tauri/src/modules/ollama/service.rs | 297 +
src-tauri/src/modules/skills/mod.rs | 2 +
src-tauri/src/modules/skills/service.rs | 869 +++
src-tauri/src/modules/skills/types.rs | 119 +
src-tauri/src/modules/tool_engine/mod.rs | 3 +
src-tauri/src/modules/tool_engine/runtime.rs | 79 +
src-tauri/src/modules/tool_engine/service.rs | 1062 +++
src-tauri/src/modules/tool_engine/types.rs | 171 +
src-tauri/src/shared/keywords.rs | 120 +
src-tauri/src/shared/mod.rs | 3 +
src-tauri/src/shared/state.rs | 141 +
src-tauri/src/shared/text.rs | 484 ++
src-tauri/tauri.conf.json | 35 +
src-tauri/tests/mcp_tools.rs | 109 +
src/App.tsx | 89 +
src/assets/pengine-logo.png | Bin 0 -> 1477815 bytes
src/index.css | 149 +
src/main.tsx | 16 +
src/modules/bot/api/index.ts | 42 +
src/modules/bot/components/SetupWizard.tsx | 257 +
src/modules/bot/components/SetupWizardSteps.tsx | 516 ++
src/modules/bot/components/TerminalPreview.tsx | 141 +
src/modules/bot/index.ts | 5 +
src/modules/bot/store/appSessionStore.ts | 42 +
src/modules/bot/types.ts | 6 +
src/modules/mcp/components/AddServerForm.tsx | 392 ++
src/modules/mcp/components/McpServerCard.tsx | 722 +++
src/modules/mcp/components/McpToolsPanel.tsx | 323 +
src/modules/mcp/index.ts | 169 +
src/modules/ollama/api/index.ts | 68 +
src/modules/ollama/index.ts | 2 +
src/modules/ollama/types.ts | 8 +
src/modules/skills/api/index.ts | 199 +
src/modules/skills/components/ClawHubBrowse.tsx | 616 ++
src/modules/skills/components/SkillsPanel.tsx | 324 +
src/modules/skills/index.ts | 3 +
src/modules/skills/types.ts | 46 +
.../toolengine/components/ToolEnginePanel.tsx | 369 ++
src/modules/toolengine/index.ts | 179 +
src/pages/DashboardPage.tsx | 229 +
src/pages/LandingPage.tsx | 400 ++
src/pages/SetupPage.tsx | 114 +
src/shared/api/config.ts | 20 +
src/shared/mcpEvents.ts | 36 +
src/shared/ui/PhoneMockup.tsx | 47 +
src/shared/ui/SpecMockup.tsx | 52 +
src/shared/ui/StyledQrCode.tsx | 20 +
src/shared/ui/TopMenu.tsx | 131 +
src/shared/ui/WizardLayout.tsx | 116 +
src/shared/useRegistryChanged.ts | 10 +
src/shared/workspaceMounts.ts | 41 +
src/styles/utilities.css | 32 +
src/vite-env.d.ts | 1 +
tools/build-local-images.sh | 62 +
tools/fetch/Dockerfile | 11 +
tools/file-manager/Dockerfile | 14 +
tools/git/Dockerfile | 15 +
tools/mcp-tools.json | 257 +
tools/memory/Dockerfile | 14 +
tools/sequential-thinking/Dockerfile | 13 +
tools/skills/weather/README.md | 53 +
tools/skills/weather/mandatory.md | 1 +
tools/time/Dockerfile | 11 +
tools/update-upstream.sh | 131 +
tsconfig.json | 25 +
tsconfig.node.json | 11 +
vite.config.ts | 26 +
178 files changed, 28599 insertions(+)
Script executed:
# Get more detailed view of the change to pengine/file-manager section
git show 73cd07a tools/mcp-tools.json | grep -A 30 -B 5 "pengine/file-manager"Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
+ "catalog_revision": 7,
+ "valid_until": "2026-05-14T00:00:00Z",
+ "minimum_pengine_version": "0.5.0",
+ "tools": [
+ {
+ "id": "pengine/file-manager",
+ "name": "File Manager",
+ "description": "Filesystem MCP in a container. Add folders in MCP Tools; each mounts at /app/<folder-name>. Install works before any folder is set.",
+ "image": "ghcr.io/pengine-ai/pengine-file-manager",
+ "current": "0.1.0",
+ "versions": [
+ {
+ "version": "0.1.0",
+ "digest": "sha256:placeholder",
+ "released_at": "2026-04-12T00:00:00Z",
+ "yanked": false,
+ "revoked": false,
+ "security": false
+ }
+ ],
+ "container_read_only_rootfs": false,
+ "mount_read_only": false,
+ "mount_workspace": true,
+ "append_workspace_roots": true,
+ "direct_return": true,
+ "upstream_mcp_npm": {
+ "package": "@modelcontextprotocol/server-filesystem",
+ "version": "2026.1.14"
+ },
+ "mcp_server_cmd": [],
+ "commands": [
+ { "name": "read_text_file", "description": "Read a file as UTF-8 text; optional head/tail line limits" },
+ { "name": "read_media_file", "description": "Read image or audio as base64 with MIME type" },
+ { "name": "read_multiple_files", "description": "Read several files in one call" },
+ { "name": "write_file", "description": "Create or overwrite a file" },
+ { "name": "edit_file", "description": "Pattern-based selective edits with optional dry run" },
Script executed:
# Search for catalog_revision usage in code
rg "catalog_revision" -B 3 -A 3Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
tools/mcp-tools.json-{
tools/mcp-tools.json- "schema_version": 1,
tools/mcp-tools.json- "generated_at": "2026-04-14T00:00:00Z",
tools/mcp-tools.json: "catalog_revision": 7,
tools/mcp-tools.json- "valid_until": "2026-05-14T00:00:00Z",
tools/mcp-tools.json- "minimum_pengine_version": "0.5.0",
tools/mcp-tools.json- "tools": [
--
src-tauri/src/modules/tool_engine/types.rs- #[serde(default)]
src-tauri/src/modules/tool_engine/types.rs- pub generated_at: String,
src-tauri/src/modules/tool_engine/types.rs- #[serde(default)]
src-tauri/src/modules/tool_engine/types.rs: pub catalog_revision: u64,
src-tauri/src/modules/tool_engine/types.rs- #[serde(default)]
src-tauri/src/modules/tool_engine/types.rs- pub valid_until: String,
src-tauri/src/modules/tool_engine/types.rs- #[serde(default)]
--
src-tauri/src/modules/tool_engine/service.rs- if let Some(cat) = try_load_local_tools_catalog() {
src-tauri/src/modules/tool_engine/service.rs- log::info!(
src-tauri/src/modules/tool_engine/service.rs- "using local tools/mcp-tools.json (revision {}); remote fetch skipped",
src-tauri/src/modules/tool_engine/service.rs: cat.catalog_revision
src-tauri/src/modules/tool_engine/service.rs- );
src-tauri/src/modules/tool_engine/service.rs- return Ok(cat);
src-tauri/src/modules/tool_engine/service.rs- }
src-tauri/src/modules/tool_engine/service.rs-
src-tauri/src/modules/tool_engine/service.rs- match fetch_remote_catalog().await {
src-tauri/src/modules/tool_engine/service.rs- Ok(cat) => {
src-tauri/src/modules/tool_engine/service.rs: log::info!("using remote catalog (revision {})", cat.catalog_revision);
src-tauri/src/modules/tool_engine/service.rs- Ok(cat)
src-tauri/src/modules/tool_engine/service.rs- }
src-tauri/src/modules/tool_engine/service.rs- Err(e) => {
Script executed:
# Look for upgrade or version checking logic
rg "upgrade\|catalog.*revision" -lRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 1
Script executed:
# Check for release notes or changelog
find . -iname "*change*" -o -iname "*release*" -o -iname "*news*" | grep -v target | grep -v node_modules | head -10Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
./src/shared/useRegistryChanged.ts
Script executed:
# Check if there's logic that uses catalog_revision to determine if tools should update
rg -B 5 -A 5 "catalog_revision" -t rust -t javascript -t typescriptRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 2
stderr:
unrecognized file type: javascript
Script executed:
# Read useRegistryChanged.ts to understand upgrade handling
cat src/shared/useRegistryChanged.tsRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
import { useEffect } from "react";
import { PENGINE_MCP_REGISTRY_CHANGED } from "./mcpEvents";
/** Re-run `callback` whenever the MCP registry changes (install, uninstall, config edit). */
export function useRegistryChanged(callback: () => void): void {
useEffect(() => {
window.addEventListener(PENGINE_MCP_REGISTRY_CHANGED, callback);
return () => window.removeEventListener(PENGINE_MCP_REGISTRY_CHANGED, callback);
}, [callback]);
}
Script executed:
# Search for where catalog revisions are compared or stored
rg "catalog_revision" -B 10 -A 10Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
tools/mcp-tools.json-{
tools/mcp-tools.json- "schema_version": 1,
tools/mcp-tools.json- "generated_at": "2026-04-14T00:00:00Z",
tools/mcp-tools.json: "catalog_revision": 7,
tools/mcp-tools.json- "valid_until": "2026-05-14T00:00:00Z",
tools/mcp-tools.json- "minimum_pengine_version": "0.5.0",
tools/mcp-tools.json- "tools": [
tools/mcp-tools.json- {
tools/mcp-tools.json- "id": "pengine/file-manager",
tools/mcp-tools.json- "name": "File Manager",
tools/mcp-tools.json- "description": "Filesystem MCP in a container. Add folders in MCP Tools; each mounts at /app/<folder-name>. Install works before any folder is set.",
tools/mcp-tools.json- "image": "ghcr.io/pengine-ai/pengine-file-manager",
tools/mcp-tools.json- "current": "0.1.0",
tools/mcp-tools.json- "versions": [
--
src-tauri/src/modules/tool_engine/types.rs- }
src-tauri/src/modules/tool_engine/types.rs-}
src-tauri/src/modules/tool_engine/types.rs-
src-tauri/src/modules/tool_engine/types.rs-/// Root of the versioned tool catalog (`tools.json`).
src-tauri/src/modules/tool_engine/types.rs-#[derive(Debug, Clone, Serialize, Deserialize)]
src-tauri/src/modules/tool_engine/types.rs-pub struct ToolCatalog {
src-tauri/src/modules/tool_engine/types.rs- pub schema_version: u32,
src-tauri/src/modules/tool_engine/types.rs- #[serde(default)]
src-tauri/src/modules/tool_engine/types.rs- pub generated_at: String,
src-tauri/src/modules/tool_engine/types.rs- #[serde(default)]
src-tauri/src/modules/tool_engine/types.rs: pub catalog_revision: u64,
src-tauri/src/modules/tool_engine/types.rs- #[serde(default)]
src-tauri/src/modules/tool_engine/types.rs- pub valid_until: String,
src-tauri/src/modules/tool_engine/types.rs- #[serde(default)]
src-tauri/src/modules/tool_engine/types.rs- pub minimum_pengine_version: String,
src-tauri/src/modules/tool_engine/types.rs- pub tools: Vec<ToolEntry>,
src-tauri/src/modules/tool_engine/types.rs-}
--
src-tauri/src/modules/tool_engine/service.rs- );
src-tauri/src/modules/tool_engine/service.rs- }
src-tauri/src/modules/tool_engine/service.rs- None
src-tauri/src/modules/tool_engine/service.rs-}
src-tauri/src/modules/tool_engine/service.rs-
src-tauri/src/modules/tool_engine/service.rs-/// Local file, then remote URL, then embedded `mcp-tools.json`.
src-tauri/src/modules/tool_engine/service.rs-pub async fn load_catalog() -> Result<ToolCatalog, String> {
src-tauri/src/modules/tool_engine/service.rs- if let Some(cat) = try_load_local_tools_catalog() {
src-tauri/src/modules/tool_engine/service.rs- log::info!(
src-tauri/src/modules/tool_engine/service.rs- "using local tools/mcp-tools.json (revision {}); remote fetch skipped",
src-tauri/src/modules/tool_engine/service.rs: cat.catalog_revision
src-tauri/src/modules/tool_engine/service.rs- );
src-tauri/src/modules/tool_engine/service.rs- return Ok(cat);
src-tauri/src/modules/tool_engine/service.rs- }
src-tauri/src/modules/tool_engine/service.rs-
src-tauri/src/modules/tool_engine/service.rs- match fetch_remote_catalog().await {
src-tauri/src/modules/tool_engine/service.rs- Ok(cat) => {
src-tauri/src/modules/tool_engine/service.rs: log::info!("using remote catalog (revision {})", cat.catalog_revision);
src-tauri/src/modules/tool_engine/service.rs- Ok(cat)
src-tauri/src/modules/tool_engine/service.rs- }
src-tauri/src/modules/tool_engine/service.rs- Err(e) => {
src-tauri/src/modules/tool_engine/service.rs- log::warn!("remote catalog unavailable ({e}), using embedded fallback");
src-tauri/src/modules/tool_engine/service.rs- load_embedded_catalog()
src-tauri/src/modules/tool_engine/service.rs- }
src-tauri/src/modules/tool_engine/service.rs- }
src-tauri/src/modules/tool_engine/service.rs-}
src-tauri/src/modules/tool_engine/service.rs-
src-tauri/src/modules/tool_engine/service.rs-/// Try to fetch and parse the remote catalog.
Script executed:
# Look for localStorage or persistence of catalog state
rg "localStorage|persist|upgrade" -iRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
e2e/setup-dashboard.spec.ts: window.localStorage.setItem("pengine-device-session", JSON.stringify(state));
doc/skills.md:To tweak a **bundled** skill, click **Fork to custom** in the panel (or copy the folder manually). Edits to `tools/skills/` inside the app bundle will not persist across reinstalls.
doc/design/README.md:│ │ │ └── appSessionStore.ts # Zustand store (persisted to localStorage)
doc/design/README.md:- `src/modules/bot/store/appSessionStore.ts` — Zustand store for bot connection state, persisted to localStorage under key `pengine-device-session`.
doc/design/README.md:│ │ ├── repository.rs # File-based persistence (persist / load / clear)
doc/design/README.md:- `src-tauri/src/modules/bot/repository.rs` — Persists `ConnectionData` as JSON to a single file at `$APP_DATA/connection.json`. `clear()` uses direct `remove_file` (not existence check first) to avoid TOCTOU.
doc/design/README.md: persist to disk
doc/design/README.md:appSessionStore.connectDevice() → localStorage
src/modules/mcp/components/McpToolsPanel.tsx: const persistServerEntry = async (name: string, entry: ServerEntry) => {
src/modules/mcp/components/McpToolsPanel.tsx: const out = await persistServerEntry(name, entry);
src/modules/mcp/components/McpToolsPanel.tsx: const out = await persistServerEntry(name, entry);
src/App.tsx:/** One-shot: sync dashboard route with persisted session or running local app. */
src/App.tsx: if (useAppSessionStore.persist.hasHydrated()) {
src/App.tsx: return useAppSessionStore.persist.onFinishHydration(() => {
src/modules/bot/store/appSessionStore.ts:import { createJSONStorage, persist } from "zustand/middleware";
src/modules/bot/store/appSessionStore.ts: persist(
src/modules/bot/store/appSessionStore.ts: storage: createJSONStorage(() => localStorage),
src-tauri/src/app.rs: // Resume persisted Telegram connection if present.
src-tauri/src/modules/bot/agent.rs:// ── Background memory persistence ──────────────────────────────────
src-tauri/src/modules/mcp/service.rs:/// and workspace folders persist regardless of cwd or where the binary lives (debug or release).
src-tauri/src/modules/tool_engine/types.rs: /// path via env so the tool can persist state (e.g. the Memory server's knowledge-graph JSON).
src-tauri/src/modules/skills/service.rs:/// Mark `slug` enabled or disabled. Persisted in `.disabled.json`.
src-tauri/src/infrastructure/http_server.rs: repository::persist(&state.store_path, &conn).map_err(|e| {
src-tauri/src/modules/bot/repository.rs:pub fn persist(path: &Path, data: &ConnectionData) -> Result<(), String> {
Script executed:
# Check the service.rs file more thoroughly for catalog loading logic
cat -n src-tauri/src/modules/tool_engine/service.rs | grep -A 20 -B 5 "catalog_revision\|fetch_remote"Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
120 /// Local file, then remote URL, then embedded `mcp-tools.json`.
121 pub async fn load_catalog() -> Result<ToolCatalog, String> {
122 if let Some(cat) = try_load_local_tools_catalog() {
123 log::info!(
124 "using local tools/mcp-tools.json (revision {}); remote fetch skipped",
125 cat.catalog_revision
126 );
127 return Ok(cat);
128 }
129
130 match fetch_remote_catalog().await {
131 Ok(cat) => {
132 log::info!("using remote catalog (revision {})", cat.catalog_revision);
133 Ok(cat)
134 }
135 Err(e) => {
136 log::warn!("remote catalog unavailable ({e}), using embedded fallback");
137 load_embedded_catalog()
138 }
139 }
140 }
141
142 /// Try to fetch and parse the remote catalog.
143 async fn fetch_remote_catalog() -> Result<ToolCatalog, String> {
144 let client = reqwest::Client::builder()
145 .timeout(REMOTE_CATALOG_TIMEOUT)
146 .build()
147 .map_err(|e| format!("http client: {e}"))?;
148
149 let resp = client
150 .get(REMOTE_CATALOG_URL)
151 .send()
152 .await
153 .map_err(|e| format!("fetch: {e}"))?;
154
155 if !resp.status().is_success() {
156 return Err(format!("HTTP {}", resp.status()));
157 }
158
159 let body = resp.text().await.map_err(|e| format!("body: {e}"))?;
160 parse_catalog(&body).ok_or_else(|| "invalid or unsupported catalog schema".into())
161 }
162
163 /// Derive the `mcp.json` server key for a tool ID (e.g. `pengine/file-manager` -> `te_pengine-file-manager`).
Script executed:
# Look for any user-facing messages or notifications about upgrades
rg "upgrade\|new.*tool\|mount.*access" -i --type rust --type typescript | head -40Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stderr:
unrecognized file type: typescript
Script executed:
# Search for any risk or category field definitions in type definitions
rg "risk|category" -i --type rust -B 3 -A 3 | head -60Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
src-tauri/src/modules/mcp/native.rs- }
src-tauri/src/modules/mcp/native.rs- }),
src-tauri/src/modules/mcp/native.rs- direct_return: true,
src-tauri/src/modules/mcp/native.rs: category: None,
src-tauri/src/modules/mcp/native.rs: risk: super::types::ToolRisk::Low,
src-tauri/src/modules/mcp/native.rs- };
src-tauri/src/modules/mcp/native.rs- super::tool_metadata::apply(&mut t);
src-tauri/src/modules/mcp/native.rs- t
--
src-tauri/src/modules/mcp/native.rs- }
src-tauri/src/modules/mcp/native.rs- }),
src-tauri/src/modules/mcp/native.rs- direct_return: false,
src-tauri/src/modules/mcp/native.rs: category: None,
src-tauri/src/modules/mcp/native.rs: risk: super::types::ToolRisk::Low,
src-tauri/src/modules/mcp/native.rs- };
src-tauri/src/modules/mcp/native.rs- super::tool_metadata::apply(&mut t);
src-tauri/src/modules/mcp/native.rs- t
--
src-tauri/src/modules/mcp/tool_metadata.rs://! Heuristic `category` + `risk` for MCP tools (MCP payloads do not include these).
src-tauri/src/modules/mcp/tool_metadata.rs-
src-tauri/src/modules/mcp/tool_metadata.rs:use super::types::{ToolDef, ToolRisk};
src-tauri/src/modules/mcp/tool_metadata.rs-
src-tauri/src/modules/mcp/tool_metadata.rs-pub fn apply(tool: &mut ToolDef) {
src-tauri/src/modules/mcp/tool_metadata.rs- let n = tool.name.to_lowercase();
src-tauri/src/modules/mcp/tool_metadata.rs- let d = tool.description.as_deref().unwrap_or("").to_lowercase();
src-tauri/src/modules/mcp/tool_metadata.rs-
src-tauri/src/modules/mcp/tool_metadata.rs: let (category, risk) = classify(&n, &d);
src-tauri/src/modules/mcp/tool_metadata.rs: tool.category = Some(category);
src-tauri/src/modules/mcp/tool_metadata.rs: tool.risk = risk;
src-tauri/src/modules/mcp/tool_metadata.rs-}
src-tauri/src/modules/mcp/tool_metadata.rs-
src-tauri/src/modules/mcp/tool_metadata.rs:fn classify(name: &str, desc: &str) -> (String, ToolRisk) {
src-tauri/src/modules/mcp/tool_metadata.rs- if name == "fetch" || name.contains("http") || desc.contains("http") || desc.contains("url") {
src-tauri/src/modules/mcp/tool_metadata.rs: return ("web".into(), ToolRisk::Low);
src-tauri/src/modules/mcp/tool_metadata.rs- }
src-tauri/src/modules/mcp/tool_metadata.rs- if name == "time" || name.contains("clock") {
src-tauri/src/modules/mcp/tool_metadata.rs: return ("utility".into(), ToolRisk::Low);
src-tauri/src/modules/mcp/tool_metadata.rs- }
src-tauri/src/modules/mcp/tool_metadata.rs- if name == "roll_dice" {
src-tauri/src/modules/mcp/tool_metadata.rs: return ("utility".into(), ToolRisk::Low);
src-tauri/src/modules/mcp/tool_metadata.rs- }
src-tauri/src/modules/mcp/tool_metadata.rs- if name == "manage_tools" || desc.contains("uninstall") && desc.contains("catalog") {
src-tauri/src/modules/mcp/tool_metadata.rs: return ("system".into(), ToolRisk::High);
src-tauri/src/modules/mcp/tool_metadata.rs- }
src-tauri/src/modules/mcp/tool_metadata.rs- if matches!(
src-tauri/src/modules/mcp/tool_metadata.rs- name,
--
src-tauri/src/modules/mcp/tool_metadata.rs- ) || desc.contains("knowledge graph")
src-tauri/src/modules/mcp/tool_metadata.rs- || desc.contains("entity")
src-tauri/src/modules/mcp/tool_metadata.rs- {
src-tauri/src/modules/mcp/tool_metadata.rs: return ("memory".into(), ToolRisk::Medium);
src-tauri/src/modules/mcp/tool_metadata.rs- }
src-tauri/src/modules/mcp/tool_metadata.rs- if name.contains("search_nodes") || name.contains("open_nodes") || name.contains("read_graph") {
src-tauri/src/modules/mcp/tool_metadata.rs: return ("memory".into(), ToolRisk::Low);
src-tauri/src/modules/mcp/tool_metadata.rs- }
src-tauri/src/modules/mcp/tool_metadata.rs- if name.contains("write")
src-tauri/src/modules/mcp/tool_metadata.rs- || name.contains("delete")
--
src-tauri/src/modules/mcp/tool_metadata.rs- || name.contains("run_terminal")
src-tauri/src/modules/mcp/tool_metadata.rs- || name.contains("shell")
Script executed:
# Check if there's any tool filtering or gating logic based on risk
rg "select_tools|filter.*tool|gate" -i --type rust | head -40Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
src-tauri/src/infrastructure/http_server.rs: .map_err(|e| (StatusCode::BAD_GATEWAY, Json(ErrorResponse { error: e })))?;
src-tauri/src/infrastructure/http_server.rs: .map_err(|e| (StatusCode::BAD_GATEWAY, Json(ErrorResponse { error: e })))?;
src-tauri/src/infrastructure/http_server.rs: .map_err(|e| (StatusCode::BAD_GATEWAY, Json(ErrorResponse { error: e })))?;
src-tauri/src/infrastructure/http_server.rs: .map_err(|e| (StatusCode::BAD_GATEWAY, Json(ErrorResponse { error: e })))?;
src-tauri/src/infrastructure/http_server.rs: .map_err(|e| (StatusCode::BAD_GATEWAY, Json(ErrorResponse { error: e })))?;
src-tauri/src/modules/bot/service.rs:/// Send failures are logged (not propagated) so one bad chunk doesn't abort
src-tauri/src/modules/bot/agent.rs: reg.select_tools_for_turn(
src-tauri/src/modules/tool_engine/service.rs:/// (host-process env does not propagate into the container).
src-tauri/src/modules/mcp/registry.rs: .filter(|t| !is_deprecated_mcp_tool(t))
src-tauri/src/modules/mcp/registry.rs: .filter(|t| !is_deprecated_mcp_tool(t))
src-tauri/src/modules/mcp/registry.rs: pub fn select_tools_for_turn(
src-tauri/src/modules/mcp/registry.rs: let high = selected.iter().filter(|t| t.risk == ToolRisk::High).count();
src-tauri/src/modules/mcp/registry.rs: let high = all.iter().filter(|t| t.risk == ToolRisk::High).count();
src-tauri/src/modules/mcp/registry.rs: let high = all.iter().filter(|t| t.risk == ToolRisk::High).count();
src-tauri/src/modules/mcp/registry.rs: .filter(|q| tool_tokens.contains(q.as_str()))
Script executed:
# Look for any changes or PRs that mention risk routing
git log --all --oneline | grep -i "risk\|category\|gate\|filter.*tool"Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
438a225 feat: add Unicode normalization and general category support
Script executed:
# Get the full tool_metadata.rs file to understand risk classification
cat src-tauri/src/modules/mcp/tool_metadata.rsRepository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
//! Heuristic `category` + `risk` for MCP tools (MCP payloads do not include these).
use super::types::{ToolDef, ToolRisk};
pub fn apply(tool: &mut ToolDef) {
let n = tool.name.to_lowercase();
let d = tool.description.as_deref().unwrap_or("").to_lowercase();
let (category, risk) = classify(&n, &d);
tool.category = Some(category);
tool.risk = risk;
}
fn classify(name: &str, desc: &str) -> (String, ToolRisk) {
if name == "fetch" || name.contains("http") || desc.contains("http") || desc.contains("url") {
return ("web".into(), ToolRisk::Low);
}
if name == "time" || name.contains("clock") {
return ("utility".into(), ToolRisk::Low);
}
if name == "roll_dice" {
return ("utility".into(), ToolRisk::Low);
}
if name == "manage_tools" || desc.contains("uninstall") && desc.contains("catalog") {
return ("system".into(), ToolRisk::High);
}
if matches!(
name,
"create_entities" | "add_observations" | "create_relations" | "delete_entities"
) || desc.contains("knowledge graph")
|| desc.contains("entity")
{
return ("memory".into(), ToolRisk::Medium);
}
if name.contains("search_nodes") || name.contains("open_nodes") || name.contains("read_graph") {
return ("memory".into(), ToolRisk::Low);
}
if name.contains("write")
|| name.contains("delete")
|| name.contains("remove")
|| name.contains("exec")
|| name.contains("run_terminal")
|| name.contains("shell")
{
return ("filesystem".into(), ToolRisk::High);
}
if name.contains("read") || name.contains("list") || name.contains("directory") {
return ("filesystem".into(), ToolRisk::Medium);
}
("other".into(), ToolRisk::Medium)
}
Script executed:
# Check how select_tools_for_turn uses the risk classification
cat -n src-tauri/src/modules/mcp/registry.rs | grep -A 40 "select_tools_for_turn"Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
193 pub fn select_tools_for_turn(
194 &self,
195 user_message: &str,
196 recent_tool_names: &[String],
197 memory_server: Option<&str>,
198 chat_session_recording: bool,
199 ) -> ToolContextSelection {
200 let t0 = Instant::now();
201 let all = self.all_tools();
202 let total = all.len();
203 match route_tools(
204 &all,
205 user_message,
206 recent_tool_names,
207 memory_server,
208 chat_session_recording,
209 ) {
210 ToolRoutePlan::Subset {
211 tools: selected,
212 routing,
213 } => {
214 let select_ms = t0.elapsed().as_millis() as u64;
215 let high = selected.iter().filter(|t| t.risk == ToolRisk::High).count();
216 let active = selected.len();
217 ToolContextSelection {
218 tools_json: build_ollama_tools(&selected),
219 total_count: total,
220 active_count: active,
221 used_subset: true,
222 routing,
223 select_ms,
224 high_risk_active: high,
225 }
226 }
227 ToolRoutePlan::FullCatalog => {
228 let select_ms = t0.elapsed().as_millis() as u64;
229 let high = all.iter().filter(|t| t.risk == ToolRisk::High).count();
230 ToolContextSelection {
231 tools_json: self.cached_ollama_tools.clone(),
232 total_count: total,
233 active_count: total,
Script executed:
# Check types.rs to see ToolRisk enum definition
rg "enum ToolRisk" -A 10Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
src-tauri/src/modules/mcp/types.rs:pub enum ToolRisk {
src-tauri/src/modules/mcp/types.rs- #[default]
src-tauri/src/modules/mcp/types.rs- Low,
src-tauri/src/modules/mcp/types.rs- Medium,
src-tauri/src/modules/mcp/types.rs- High,
src-tauri/src/modules/mcp/types.rs-}
src-tauri/src/modules/mcp/types.rs-
src-tauri/src/modules/mcp/types.rs-/// Definition of a single tool, regardless of where it runs.
src-tauri/src/modules/mcp/types.rs-#[derive(Debug, Clone, Serialize, Deserialize)]
src-tauri/src/modules/mcp/types.rs-pub struct ToolDef {
src-tauri/src/modules/mcp/types.rs- #[serde(skip)]
Script executed:
# Check what route_tools does and whether it actually gates high-risk tools
rg "route_tools|ToolRoutePlan" -B 5 -A 20 | head -100Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
src-tauri/src/modules/mcp/registry.rs- chat_session_recording: bool,
src-tauri/src/modules/mcp/registry.rs- ) -> ToolContextSelection {
src-tauri/src/modules/mcp/registry.rs- let t0 = Instant::now();
src-tauri/src/modules/mcp/registry.rs- let all = self.all_tools();
src-tauri/src/modules/mcp/registry.rs- let total = all.len();
src-tauri/src/modules/mcp/registry.rs: match route_tools(
src-tauri/src/modules/mcp/registry.rs- &all,
src-tauri/src/modules/mcp/registry.rs- user_message,
src-tauri/src/modules/mcp/registry.rs- recent_tool_names,
src-tauri/src/modules/mcp/registry.rs- memory_server,
src-tauri/src/modules/mcp/registry.rs- chat_session_recording,
src-tauri/src/modules/mcp/registry.rs- ) {
src-tauri/src/modules/mcp/registry.rs: ToolRoutePlan::Subset {
src-tauri/src/modules/mcp/registry.rs- tools: selected,
src-tauri/src/modules/mcp/registry.rs- routing,
src-tauri/src/modules/mcp/registry.rs- } => {
src-tauri/src/modules/mcp/registry.rs- let select_ms = t0.elapsed().as_millis() as u64;
src-tauri/src/modules/mcp/registry.rs- let high = selected.iter().filter(|t| t.risk == ToolRisk::High).count();
src-tauri/src/modules/mcp/registry.rs- let active = selected.len();
src-tauri/src/modules/mcp/registry.rs- ToolContextSelection {
src-tauri/src/modules/mcp/registry.rs- tools_json: build_ollama_tools(&selected),
src-tauri/src/modules/mcp/registry.rs- total_count: total,
src-tauri/src/modules/mcp/registry.rs- active_count: active,
src-tauri/src/modules/mcp/registry.rs- used_subset: true,
src-tauri/src/modules/mcp/registry.rs- routing,
src-tauri/src/modules/mcp/registry.rs- select_ms,
src-tauri/src/modules/mcp/registry.rs- high_risk_active: high,
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs: ToolRoutePlan::FullCatalog => {
src-tauri/src/modules/mcp/registry.rs- let select_ms = t0.elapsed().as_millis() as u64;
src-tauri/src/modules/mcp/registry.rs- let high = all.iter().filter(|t| t.risk == ToolRisk::High).count();
src-tauri/src/modules/mcp/registry.rs- ToolContextSelection {
src-tauri/src/modules/mcp/registry.rs- tools_json: self.cached_ollama_tools.clone(),
src-tauri/src/modules/mcp/registry.rs- total_count: total,
src-tauri/src/modules/mcp/registry.rs- active_count: total,
src-tauri/src/modules/mcp/registry.rs- used_subset: false,
src-tauri/src/modules/mcp/registry.rs- routing: "full",
src-tauri/src/modules/mcp/registry.rs- select_ms,
src-tauri/src/modules/mcp/registry.rs- high_risk_active: high,
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs-
src-tauri/src/modules/mcp/registry.rs- /// Full catalog (no subset). Used after routing escalation.
src-tauri/src/modules/mcp/registry.rs- pub fn full_tool_context(&self) -> ToolContextSelection {
src-tauri/src/modules/mcp/registry.rs- let all = self.all_tools();
src-tauri/src/modules/mcp/registry.rs- let total = self.cached_tool_names.len();
src-tauri/src/modules/mcp/registry.rs- let high = all.iter().filter(|t| t.risk == ToolRisk::High).count();
--
src-tauri/src/modules/mcp/registry.rs-/// getting stuck when keyword matching misses (short queries, non-English
src-tauri/src/modules/mcp/registry.rs-/// intent words, typos) — a tiny token cost for a big correctness win.
src-tauri/src/modules/mcp/registry.rs-const ALWAYS_ON_TOOL_NAMES: &[&str] = &["fetch", "time"];
src-tauri/src/modules/mcp/registry.rs-
src-tauri/src/modules/mcp/registry.rs-#[derive(Debug)]
src-tauri/src/modules/mcp/registry.rs:enum ToolRoutePlan {
src-tauri/src/modules/mcp/registry.rs- FullCatalog,
src-tauri/src/modules/mcp/registry.rs- Subset {
src-tauri/src/modules/mcp/registry.rs- tools: Vec<ToolDef>,
src-tauri/src/modules/mcp/registry.rs- routing: &'static str,
src-tauri/src/modules/mcp/registry.rs- },
src-tauri/src/modules/mcp/registry.rs-}
src-tauri/src/modules/mcp/registry.rs-
src-tauri/src/modules/mcp/registry.rs-fn registry_routing_threshold(tools: &[ToolDef], memory_server: Option<&str>) -> usize {
src-tauri/src/modules/mcp/registry.rs- let memory_tool_count = memory_server
src-tauri/src/modules/mcp/registry.rs- .map(|m| {
src-tauri/src/modules/mcp/registry.rs- tools
src-tauri/src/modules/mcp/registry.rs- .iter()
src-tauri/src/modules/mcp/registry.rs- .filter(|t| t.server_name.eq_ignore_ascii_case(m))
src-tauri/src/modules/mcp/registry.rs- .count()
src-tauri/src/modules/mcp/registry.rs- })
src-tauri/src/modules/mcp/registry.rs- .unwrap_or(0);
src-tauri/src/modules/mcp/registry.rs- ROUTED_TOOL_BUDGET + ALWAYS_ON_TOOL_NAMES.len() + memory_tool_count
src-tauri/src/modules/mcp/registry.rs-}
src-tauri/src/modules/mcp/registry.rs-
src-tauri/src/modules/mcp/registry.rs-fn push_always_on_tools(
--
src-tauri/src/modules/mcp/registry.rs-}
src-tauri/src/modules/mcp/registry.rs-
src-tauri/src/modules/mcp/registry.rs-/// Large registries: return a subset. Small registries (`len` ≤ threshold): pass the full list.
src-tauri/src/modules/mcp/registry.rs-/// When every tool scores 0 (common for non-English queries before any tool was used recently),
src-tauri/src/modules/mcp/registry.rs-/// use **always-on** tools only (`fetch`, `time`) — not the full catalog.
src-tauri/src/modules/mcp/registry.rs:fn route_tools(
src-tauri/src/modules/mcp/registry.rs- tools: &[ToolDef],
src-tauri/src/modules/mcp/registry.rs- user_message: &str,
src-tauri/src/modules/mcp/registry.rs- recent_tool_names: &[String],
src-tauri/src/modules/mcp/registry.rs- memory_server: Option<&str>,
src-tauri/src/modules/mcp/registry.rs- chat_session_recording: bool,
src-tauri/src/modules/mcp/registry.rs:) -> ToolRoutePlan {
src-tauri/src/modules/mcp/registry.rs- let threshold = registry_routing_threshold(tools, memory_server);
src-tauri/src/modules/mcp/registry.rs- if tools.len() <= threshold {
src-tauri/src/modules/mcp/registry.rs: return ToolRoutePlan::FullCatalog;
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs-
src-tauri/src/modules/mcp/registry.rs- let query_tokens = route_tokens(user_message);
src-tauri/src/modules/mcp/registry.rs- let mut scored: Vec<(usize, &ToolDef)> = tools
src-tauri/src/modules/mcp/registry.rs- .iter()
src-tauri/src/modules/mcp/registry.rs- .map(|t| {
src-tauri/src/modules/mcp/registry.rs- let s = score_tool_combined(t, &query_tokens, recent_tool_names, user_message);
Script executed:
# Look for any logic that blocks/prevents high-risk tool execution
rg "high_risk" -i -B 3 -A 3Repository: pengine-ai/pengine
Repository: pengine-ai/pengine
Exit code: 0
stdout:
src-tauri/src/modules/bot/agent.rs- .emit_log(
src-tauri/src/modules/bot/agent.rs- "tool_ctx",
src-tauri/src/modules/bot/agent.rs- &format!(
src-tauri/src/modules/bot/agent.rs: "select_ms={} active={}/{} subset={} routing={} recording={} high_risk={} recent_n={}",
src-tauri/src/modules/bot/agent.rs- tool_ctx.select_ms,
src-tauri/src/modules/bot/agent.rs- tool_ctx.active_count,
src-tauri/src/modules/bot/agent.rs- tool_ctx.total_count,
src-tauri/src/modules/bot/agent.rs- tool_ctx.used_subset,
src-tauri/src/modules/bot/agent.rs- tool_ctx.routing,
src-tauri/src/modules/bot/agent.rs- chat_session_recording,
src-tauri/src/modules/bot/agent.rs: tool_ctx.high_risk_active,
src-tauri/src/modules/bot/agent.rs- recent_tools.len()
src-tauri/src/modules/bot/agent.rs- ),
src-tauri/src/modules/bot/agent.rs- )
--
src-tauri/src/modules/mcp/registry.rs- /// `core_no_signal` = no scores (e.g. non-English) — always-on + memory only.
src-tauri/src/modules/mcp/registry.rs- pub routing: &'static str,
src-tauri/src/modules/mcp/registry.rs- pub select_ms: u64,
src-tauri/src/modules/mcp/registry.rs: pub high_risk_active: usize,
src-tauri/src/modules/mcp/registry.rs-}
src-tauri/src/modules/mcp/registry.rs-
src-tauri/src/modules/mcp/registry.rs-impl Default for ToolRegistry {
--
src-tauri/src/modules/mcp/registry.rs- used_subset: true,
src-tauri/src/modules/mcp/registry.rs- routing,
src-tauri/src/modules/mcp/registry.rs- select_ms,
src-tauri/src/modules/mcp/registry.rs: high_risk_active: high,
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs- ToolRoutePlan::FullCatalog => {
--
src-tauri/src/modules/mcp/registry.rs- used_subset: false,
src-tauri/src/modules/mcp/registry.rs- routing: "full",
src-tauri/src/modules/mcp/registry.rs- select_ms,
src-tauri/src/modules/mcp/registry.rs: high_risk_active: high,
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs- }
--
src-tauri/src/modules/mcp/registry.rs- used_subset: false,
src-tauri/src/modules/mcp/registry.rs- routing: "full_escalation",
src-tauri/src/modules/mcp/registry.rs- select_ms: 0,
src-tauri/src/modules/mcp/registry.rs: high_risk_active: high,
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs- }
src-tauri/src/modules/mcp/registry.rs-
Security posture change: File Manager mounts are now read-write by default.
Flipping mount_read_only from true to false for pengine/file-manager grants the containerized MCP server write/delete access to every user-configured workspace root. The write_file, edit_file, move_file, and create_directory commands are all present and unrestricted, exposing the system to silent file mutations if the model is prompt-injected or compromised.
Critical gaps found:
- No user notification: There is no upgrade prompt or install warning informing users that this tool now has write access to their workspace folders.
- No catalog upgrade mechanism: While
catalog_revision: 7exists, there is no logic comparing revisions or prompting existing users on updates; they may be silently upgraded. - Risk classification exists but does not gate execution: The codebase includes a
riskclassification system that marks filesystem write operations asToolRisk::High, but this is only used for logging metrics. These high-risk commands are not blocked, gated behind user confirmation, or surfaced differently to the model—they execute unconditionally alongside lower-risk operations.
Please confirm this is intentional and address the notification and gating gaps before release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/mcp-tools.json` at line 25, The change makes pengine/file-manager
writable by default (mount_read_only -> false) which exposes write/delete
commands (write_file, edit_file, move_file, create_directory) without
notification or gating; revert mount_read_only to true for now and implement:
(1) an upgrade/installation prompt that surfaces catalog_revision to existing
users before changing behavior, (2) a version-check/upgrade path that compares
catalog_revision and prompts users to accept the new write capability, and (3)
enforce risk gating so ToolRisk::High operations are either blocked or require
explicit user confirmation (and surfaced differently to the model/controller)
rather than executing unconditionally. Ensure the changes reference the
pengine/file-manager config, catalog_revision, and the high-risk command names
so code paths can be located and tested.
- Changed all references from README.md to SKILL.md in the skills documentation to standardize file naming. - Enhanced the SkillsPanel component to support editing and saving skills with the new SKILL.md format. - Updated the skills service to handle SKILL.md files for reading, writing, and validation. - Introduced a new weather skill example in SKILL.md format, providing detailed usage instructions and fallback mechanisms.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/modules/bot/agent.rs (1)
779-787:⚠️ Potential issue | 🟡 MinorSummary fallback silently drops all but the last tool output.
When the phase-2 summarization returns empty text, only
tool_results.into_iter().last().unwrap().1is returned to the user. If multiple tools ran (e.g. sitemap + severalfetchs), the earlier results are lost without any indication. Concatenating (or at least joining with the existing--- {name} ---separator used at line 741) would preserve the information the user's turn gathered.🛠️ Possible fix
- let fallback = tool_results.into_iter().last().unwrap().1; + let fallback = tool_results + .into_iter() + .map(|(name, content)| format!("--- {name} ---\n{content}")) + .collect::<Vec<_>>() + .join("\n\n"); return Ok(TurnResult { text: fallback, source: ReplySource::Tool, suppress_telegram_reply: false, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/bot/agent.rs` around lines 779 - 787, The current fallback logic takes only the last tool output (tool_results.into_iter().last().unwrap().1) and returns it as TurnResult, losing earlier tool outputs; instead, when the phase-2 summary is empty, iterate tool_results in order and join all outputs into a single string using the same separator format used earlier (e.g. "--- {name} ---\n") so each tool's name and content are preserved, then set that concatenated string as TurnResult.text (keeping ReplySource::Tool and suppress_telegram_reply as before); update the code around the fallback variable and TurnResult construction to build and use this joined string rather than only the last entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src-tauri/src/modules/bot/agent.rs`:
- Around line 779-787: The current fallback logic takes only the last tool
output (tool_results.into_iter().last().unwrap().1) and returns it as
TurnResult, losing earlier tool outputs; instead, when the phase-2 summary is
empty, iterate tool_results in order and join all outputs into a single string
using the same separator format used earlier (e.g. "--- {name} ---\n") so each
tool's name and content are preserved, then set that concatenated string as
TurnResult.text (keeping ReplySource::Tool and suppress_telegram_reply as
before); update the code around the fallback variable and TurnResult
construction to build and use this joined string rather than only the last
entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9728d897-40d3-4b9c-8b34-25286ae059c2
📒 Files selected for processing (6)
doc/skills.mdsrc-tauri/src/modules/bot/agent.rssrc-tauri/src/modules/skills/service.rssrc-tauri/src/modules/skills/types.rssrc/modules/skills/components/SkillsPanel.tsxtools/skills/weather/SKILL.md
✅ Files skipped from review due to trivial changes (1)
- tools/skills/weather/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/modules/skills/components/SkillsPanel.tsx
- Updated the `ClawHubSearchQuery` struct to use camelCase for serialization, enhancing consistency with API standards. - Refactored the `chat_options_for_agent_step` function to better manage tool result rounds, improving clarity in the agent's decision-making process. - Adjusted the `TELEGRAM_CHUNK_BUDGET` constant to account for UTF-16 code units, ensuring compliance with Telegram's message limits. - Enhanced the `recent_tool_score` function to correctly weight recent tool invocations, optimizing tool selection based on usage history. - Expanded the tool classification logic to include additional tool types, improving the accuracy of tool risk assessments. - Cleaned up unused constants in the keywords module, streamlining the codebase.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src-tauri/src/infrastructure/http_server.rs (1)
1290-1298: Consider adding#[serde(rename_all = "camelCase")]for future-proofing.Current fields (
q,limit,cursor) are single-word so no mismatch today, but adding#[serde(rename_all = "camelCase")]now (to mirrorClawHubSearchQueryandClawHubPluginsListDto) prevents the same silent-deserialization footgun if a multi-word field (e.g.nextCursor,pageSize) is ever added. Purely optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/infrastructure/http_server.rs` around lines 1290 - 1298, Add the serde rename-all attribute to the ClawHubPluginsQuery struct to ensure future field names are deserialized using camelCase; update the struct declaration for ClawHubPluginsQuery by adding #[serde(rename_all = "camelCase")] above it (similar to ClawHubSearchQuery and ClawHubPluginsListDto) so any future multi-word fields like nextCursor or pageSize will map correctly from camelCase JSON.src-tauri/src/modules/bot/agent.rs (3)
116-127: Minor:/think//nothinkparser depends on iteration order.Listing
/nothinkbefore/thinkin the array is load-bearing — if the order were flipped,/nothink hiwould match/thinkwithrest = "nothink hi",at_boundaryis false (nisn't whitespace), so the second branch would bail and the outerforloop would try/nothink. Actually that works correctly because of the boundary check… but a future refactor that drops the boundary check or reorders entries would silently break it.A small comment, or inverting the check (
/thinkfirst but reject when next char is part of "nothink") isn't worth the churn; just worth a note. No change required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/bot/agent.rs` around lines 116 - 127, The parse_think_override function relies on the iteration order of the prefix array (["/nothink", "/think"]) to avoid accidental partial matches; add a brief explanatory comment above parse_think_override that states the current order is intentional, why the order/boundary check matters, and that future maintainers should preserve the order or switch to explicit matching logic (e.g., exact prefix checks) if refactoring; reference parse_think_override and the prefix tuple array in the comment so the intent is clear.
445-494: KV-cache ordering comment is on the call site but the fragment order lives here.The comment at line 553–555 in
run_model_turnexplains the system/user ordering, butbuild_system_promptis where the fragment order (PENGINE_OUTPUT_CONTRACT_LEAD→ assistant lead →fs_hint→mem_hint→skills_hint) is actually decided. Worth mirroring the KV-cache reuse note here as well so a future contributor doesn't reorder fragments without realizing it invalidates the prefix cache every turn.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/bot/agent.rs` around lines 445 - 494, Add a short comment inside build_system_prompt (near the format! that composes the system prompt) that mirrors the KV-cache reuse warning from run_model_turn: explicitly state that the fragment order (PENGINE_OUTPUT_CONTRACT_LEAD → assistant lead text → fs_hint → mem_hint → skills_hint) must remain stable across turns because reordering these fragments will invalidate the model prefix/KV-cache reuse; mention run_model_turn as the related call site so future editors know to update both places if they must change ordering.
795-797: Nit: likely-unreachableMAX_STEPS exceedederror.The only way out of the
forloop without returning is hittingMAX_STEPSor thebreakat line 649. After the break,tool_resultsis guaranteed non-empty (the break sits behindif tool_results.is_empty() { return … }). Any loop iteration that pushes nothing totool_resultsreturns early. Sotool_results.is_empty()when reaching line 795 should be impossible, making the error arm dead code. Either drop it in favor ofunreachable!()/a log, or fold the phase-2 block intoOk(…)and let the compiler prove the exhaustiveness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/bot/agent.rs` around lines 795 - 797, The final Err arm that returns Err(format!("agent exceeded {MAX_STEPS} steps without finishing")) is unreachable because the loop either returns when tool_results.is_empty() or breaks only after tool_results is non-empty; replace this dead error arm with unreachable!() (or remove it and fold the phase-2 result into the surrounding Ok(...) so the compiler proves exhaustiveness). Update the code around the for loop that manipulates tool_results and uses MAX_STEPS to use unreachable!() (or integrate the phase-2 block into the Ok return) to eliminate the dead/error path.src-tauri/src/shared/state.rs (1)
90-105: Minor: O(n) eviction viaVec::remove(0).With
RECENT_TOOLS_CAP = 32andTOOL_CTX_LATENCY_CAP = 128the cost is negligible, but aVecDequewithpush_back/pop_frontwould make bothnote_tools_usedandrecord_tool_selection_msstrictly O(1) per sample and make intent clearer. Feel free to defer; this is just a drive-by note.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/shared/state.rs` around lines 90 - 105, Replace the Vec-based FIFO with a VecDeque to avoid O(n) removal from the front: change the type of recent_tool_names to VecDeque<String>, update any initialization to VecDeque::new(), and in note_tools_used use push_back for each non-empty trimmed name and pop_front while len() > RECENT_TOOLS_CAP; also update record_tool_selection_ms (and any other places touching recent_tool_names) to use push_back/pop_front semantics. Keep RECENT_TOOLS_CAP unchanged and ensure all await/.lock() usages remain the same.src-tauri/src/modules/mcp/registry.rs (1)
693-737: Nit:shorten_tool_descriptionhas a redundant first pass.The first loop over
char_indices().take(cap)records acutvalue that is always overwritten whenchars().count() > cap(the.nth(cap)branch runs), and the earlychars().count() <= capbranch returns before using it. The first loop is dead work.♻️ Suggested simplification
- if trimmed.chars().count() <= cap { - return trimmed.to_string(); - } - - let mut cut = 0; - for (i, _) in trimmed.char_indices().take(cap) { - cut = i; - } - // Advance one more char so `cut` is an *exclusive* end-index. - if let Some((i, _)) = trimmed.char_indices().nth(cap) { - cut = i; - } - let base = trimmed[..cut].trim_end(); - format!("{base}…") + match trimmed.char_indices().nth(cap) { + Some((cut, _)) => { + let base = trimmed[..cut].trim_end(); + format!("{base}…") + } + None => trimmed.to_string(), + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/mcp/registry.rs` around lines 693 - 737, shorten_tool_description currently does a redundant first loop over trimmed.char_indices().take(cap) to compute `cut` which is then overwritten by the `.nth(cap)` branch or never used when `trimmed.chars().count() <= cap`; remove the dead first loop and compute the exclusive byte cut directly by attempting `trimmed.char_indices().nth(cap)` (use its index when present) and otherwise fall back to trimming to the full string (the existing early-return `if trimmed.chars().count() <= cap { ... }` already covers the full-length case), keeping the later `base = trimmed[..cut].trim_end()` and `format!("{base}…")` behavior intact so the function still returns a char-boundary ellipsized string; reference symbols: shorten_tool_description, trimmed, cap, cut, and first_sentence_end.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/modules/bot/agent.rs`:
- Around line 143-149: The status string produced for memory_hint contains
leading spaces in the Some(name) and Some(name) if diary_active branches,
causing a double-space when concatenated into the final format! call; update the
string literals in the branches that use name (the ones referencing diary_active
and the session-active message) to remove the leading space so they produce e.g.
"Diary ACTIVE (`{name}`)." and "Session ACTIVE (`{name}`); host saves each turn
— no write tools." (leave None => String::new() unchanged) so the final
format!("\nMemory server ready. Use read tools for prior context.{status}")
yields correct spacing.
In `@src-tauri/src/shared/state.rs`:
- Around line 107-127: The p95 logging stops when buf.len() saturates because
record_tool_selection_ms() uses buf.len() for the "every 10 samples" trigger;
change to a dedicated monotonically increasing counter (e.g.
samples_since_last_log or total_tool_ctx_samples) stored alongside
tool_ctx_latency_ms, increment it on each call to record_tool_selection_ms(),
and use that counter (samples % 10 == 0) to decide when to compute and emit the
p95 via emit_log("tool_ctx", ...); reset the counter when you emit. Keep
TOOL_CTX_LATENCY_CAP and tool_ctx_latency_ms logic for eviction unchanged.
---
Nitpick comments:
In `@src-tauri/src/infrastructure/http_server.rs`:
- Around line 1290-1298: Add the serde rename-all attribute to the
ClawHubPluginsQuery struct to ensure future field names are deserialized using
camelCase; update the struct declaration for ClawHubPluginsQuery by adding
#[serde(rename_all = "camelCase")] above it (similar to ClawHubSearchQuery and
ClawHubPluginsListDto) so any future multi-word fields like nextCursor or
pageSize will map correctly from camelCase JSON.
In `@src-tauri/src/modules/bot/agent.rs`:
- Around line 116-127: The parse_think_override function relies on the iteration
order of the prefix array (["/nothink", "/think"]) to avoid accidental partial
matches; add a brief explanatory comment above parse_think_override that states
the current order is intentional, why the order/boundary check matters, and that
future maintainers should preserve the order or switch to explicit matching
logic (e.g., exact prefix checks) if refactoring; reference parse_think_override
and the prefix tuple array in the comment so the intent is clear.
- Around line 445-494: Add a short comment inside build_system_prompt (near the
format! that composes the system prompt) that mirrors the KV-cache reuse warning
from run_model_turn: explicitly state that the fragment order
(PENGINE_OUTPUT_CONTRACT_LEAD → assistant lead text → fs_hint → mem_hint →
skills_hint) must remain stable across turns because reordering these fragments
will invalidate the model prefix/KV-cache reuse; mention run_model_turn as the
related call site so future editors know to update both places if they must
change ordering.
- Around line 795-797: The final Err arm that returns Err(format!("agent
exceeded {MAX_STEPS} steps without finishing")) is unreachable because the loop
either returns when tool_results.is_empty() or breaks only after tool_results is
non-empty; replace this dead error arm with unreachable!() (or remove it and
fold the phase-2 result into the surrounding Ok(...) so the compiler proves
exhaustiveness). Update the code around the for loop that manipulates
tool_results and uses MAX_STEPS to use unreachable!() (or integrate the phase-2
block into the Ok return) to eliminate the dead/error path.
In `@src-tauri/src/modules/mcp/registry.rs`:
- Around line 693-737: shorten_tool_description currently does a redundant first
loop over trimmed.char_indices().take(cap) to compute `cut` which is then
overwritten by the `.nth(cap)` branch or never used when
`trimmed.chars().count() <= cap`; remove the dead first loop and compute the
exclusive byte cut directly by attempting `trimmed.char_indices().nth(cap)` (use
its index when present) and otherwise fall back to trimming to the full string
(the existing early-return `if trimmed.chars().count() <= cap { ... }` already
covers the full-length case), keeping the later `base =
trimmed[..cut].trim_end()` and `format!("{base}…")` behavior intact so the
function still returns a char-boundary ellipsized string; reference symbols:
shorten_tool_description, trimmed, cap, cut, and first_sentence_end.
In `@src-tauri/src/shared/state.rs`:
- Around line 90-105: Replace the Vec-based FIFO with a VecDeque to avoid O(n)
removal from the front: change the type of recent_tool_names to
VecDeque<String>, update any initialization to VecDeque::new(), and in
note_tools_used use push_back for each non-empty trimmed name and pop_front
while len() > RECENT_TOOLS_CAP; also update record_tool_selection_ms (and any
other places touching recent_tool_names) to use push_back/pop_front semantics.
Keep RECENT_TOOLS_CAP unchanged and ensure all await/.lock() usages remain the
same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1134ea32-f1bb-4eef-bf30-e942a5862c56
📒 Files selected for processing (7)
src-tauri/src/infrastructure/http_server.rssrc-tauri/src/modules/bot/agent.rssrc-tauri/src/modules/bot/service.rssrc-tauri/src/modules/mcp/registry.rssrc-tauri/src/modules/mcp/tool_metadata.rssrc-tauri/src/modules/ollama/keywords.rssrc-tauri/src/shared/state.rs
✅ Files skipped from review due to trivial changes (2)
- src-tauri/src/modules/mcp/tool_metadata.rs
- src-tauri/src/modules/bot/service.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src-tauri/src/modules/ollama/keywords.rs
Comment on lines
+143
to
+149
| Some(name) if diary_active => format!(" Diary ACTIVE (`{name}`)."), | ||
| Some(name) => { | ||
| format!(" Session ACTIVE (`{name}`); host saves each turn — no write tools.") | ||
| } | ||
| Some(name) => format!( | ||
| " Chat session ACTIVE (`{name}`); host records each turn — do NOT call memory write tools." | ||
| ), | ||
| None => String::new(), | ||
| }; | ||
| format!( | ||
| "\nMemory MCP server connected. Host controls recording via keywords \ | ||
| (\"captain's log\" / \"record\" to start; \"close session\" / \"over and out\" / \ | ||
| \"Commander <name> out\" to stop). Use read tools (`read_graph`, `search_nodes`, \ | ||
| `open_nodes`) when prior context helps.{status}" | ||
| ) | ||
| format!("\nMemory server ready. Use read tools for prior context.{status}") |
There was a problem hiding this comment.
Minor: tiny formatting nit in memory_hint.
The Some(name) if diary_active branch returns " Diary ACTIVE (\{name}`)."with a leading space, but the wrappingformat!("\nMemory server ready. Use read tools for prior context.{status}")already precedes it with a sentence-ending., producing a double-space (". "+" Diary"). Not visible to users, only to the model, but it slightly bloats the prompt. Consider stripping the leading space on the statusstrings for consistency withString::new()`.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/modules/bot/agent.rs` around lines 143 - 149, The status string
produced for memory_hint contains leading spaces in the Some(name) and
Some(name) if diary_active branches, causing a double-space when concatenated
into the final format! call; update the string literals in the branches that use
name (the ones referencing diary_active and the session-active message) to
remove the leading space so they produce e.g. "Diary ACTIVE (`{name}`)." and
"Session ACTIVE (`{name}`); host saves each turn — no write tools." (leave None
=> String::new() unchanged) so the final format!("\nMemory server ready. Use
read tools for prior context.{status}") yields correct spacing.
Comment on lines
+107
to
127
| pub async fn record_tool_selection_ms(&self, ms: u64) { | ||
| let mut buf = self.tool_ctx_latency_ms.lock().await; | ||
| buf.push(ms); | ||
| while buf.len() > TOOL_CTX_LATENCY_CAP { | ||
| buf.remove(0); | ||
| } | ||
| let n = buf.len(); | ||
| if n >= 10 && n % 10 == 0 { | ||
| let mut s = buf.clone(); | ||
| s.sort_unstable(); | ||
| let idx = ((n as f64 * 0.95).ceil() as usize) | ||
| .saturating_sub(1) | ||
| .min(n - 1); | ||
| let p95 = s[idx]; | ||
| self.emit_log( | ||
| "tool_ctx", | ||
| &format!("p95_select_ms={p95} samples={n} (routing histogram)"), | ||
| ) | ||
| .await; | ||
| } | ||
| } |
There was a problem hiding this comment.
p95 logging stops firing once the latency buffer saturates.
Once buf.len() reaches TOOL_CTX_LATENCY_CAP (128), it stays at 128 for every subsequent call. The n % 10 == 0 check fails at n=128 (128 % 10 == 8), so no further p95 samples are emitted — the routing histogram log effectively dies after the first ~128 selections. Consider tracking a separate monotonically increasing counter for "samples since last log" instead of using buf.len(), or picking a cap that is a multiple of 10.
🛠️ Proposed fix sketch
- let n = buf.len();
- if n >= 10 && n % 10 == 0 {
+ let n = buf.len();
+ // Use a fixed cap that is a multiple of 10 so the histogram keeps
+ // firing once the rolling window is full.
+ if n >= 10 && n % 10 == 0 {Simpler: make TOOL_CTX_LATENCY_CAP a multiple of 10 (e.g. 120) and/or log every N pushes via a dedicated counter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/shared/state.rs` around lines 107 - 127, The p95 logging stops
when buf.len() saturates because record_tool_selection_ms() uses buf.len() for
the "every 10 samples" trigger; change to a dedicated monotonically increasing
counter (e.g. samples_since_last_log or total_tool_ctx_samples) stored alongside
tool_ctx_latency_ms, increment it on each call to record_tool_selection_ms(),
and use that counter (samples % 10 == 0) to decide when to compute and emit the
p95 via emit_log("tool_ctx", ...); reset the counter when you emit. Keep
TOOL_CTX_LATENCY_CAP and tool_ctx_latency_ms logic for eviction unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Documentation