chore: improve error messages and documentation#66
chore: improve error messages and documentation#66vlordier wants to merge 6 commits intoLiquid4All:mainfrom
Conversation
- Tool Result Compression: smart summarization for large tool outputs (directory listings, search results, JSON data) - Request Deduplication: 500ms debounce on send button to prevent duplicate requests from rapid clicks - Config Hot Reload: poll config file for changes, reload without restart, show toast notification - Error Boundary: timeout wrapper (120s) around tool execution, graceful error messages instead of crashing agent loop
- Add 13 tests for settings.rs (AppSettings, SamplingConfig, config hot reload) - Add 12 tests for chat.rs compression functions (truncate, compress directory, search, JSON) - Total tests: 392 (up from 365) - Coverage: 42.5% (up from 39.5%)
- Add ModelStatus tests (serialization, healthy/unhealthy states) - Add SamplingOverrides serialization tests - Add InferenceClient tests for: - LM Studio URL construction and model selection - Tool call format (NativeJson vs Pythonic) - Fallback chain exhaustion (AllModelsUnavailable) - is_retriable for various HTTP status codes - Error repair from malformed tool calls - Total tests: 417 (up from 392)
- Add tests for data_dir(), cache_dir(), resolve_db_path() - Add rotate_log_file() tests (creates rotated copies, handles missing files) - Add filter_by_enabled_servers() tests (filters correctly, handles missing/invalid config) - Add resolve_vision_model() tests (returns None without config) - Add filter_tools_by_allowlist() test (doesn't panic without config) - Add load_override_file() tests (parses config, returns empty for missing) - Total tests: 430 (up from 417) - Coverage: 43.3% (up from 42.5%)
- Add compression_threshold_chars and max_tool_result_chars to SamplingConfig - Add version field to AppSettings with migration support - Re-export AppSettings and SamplingConfig from lib.rs - Add helper methods for accessing compression config This enables future UI-based configuration of tool result compression.
- Add troubleshooting hints to error messages (directory permissions, config) - Improve docstring for run() function explaining initialization steps - More descriptive logging for database and Tauri startup failures
There was a problem hiding this comment.
Pull request overview
This PR improves the app’s troubleshooting experience by expanding actionable error context, adding/expanding inline documentation, and introducing supporting infrastructure across the frontend (settings UX/config watching) and the Tauri backend (MCP client, inference, and provisioning commands).
Changes:
- Adds a Settings store + config auto-reload notification UX in the React app.
- Introduces/expands Tauri backend modules for MCP server management, inference streaming/parsing, and provisioning/downloading commands with structured errors.
- Adds extensive doc/comments and tests around several new Rust modules to make failures more diagnosable.
Reviewed changes
Copilot reviewed 48 out of 55 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/stores/settingsStore.ts | Adds Zustand settings store, including config reload polling + notification state. |
| src/components/Chat/MessageInput.tsx | Adds message input component with send debouncing and autosizing. |
| src/App.tsx | Wires settings panel + config reload toast + starts/stops config watch. |
| src-tauri/tauri.conf.json | Adds Tauri v2 config, CSP, bundling, and window settings. |
| src-tauri/src/mcp_client/types.rs | Defines MCP JSON-RPC + protocol types and tests. |
| src-tauri/src/mcp_client/transport.rs | Implements stdio JSON-RPC transport + result extraction + tests. |
| src-tauri/src/mcp_client/mod.rs | Exposes MCP client modules and re-exports. |
| src-tauri/src/mcp_client/lifecycle.rs | Adds server spawn/restart/shutdown with stderr diagnostics + timeouts. |
| src-tauri/src/mcp_client/errors.rs | Defines MCP client error enum with contextual messages. |
| src-tauri/src/mcp_client/discovery.rs | Adds MCP server auto-discovery + merge behavior + tests. |
| src-tauri/src/mcp_client/client.rs | Adds high-level MCP client orchestration + tool-call execution + tests. |
| src-tauri/src/main.rs | Adds Tauri entry point calling localcowork::run(). |
| src-tauri/src/inference/types.rs | Defines OpenAI-compatible inference types + serialization rules + tests. |
| src-tauri/src/inference/streaming.rs | Adds SSE/non-stream parsing with tool-call accumulation + tests. |
| src-tauri/src/inference/mod.rs | Exposes inference modules and re-exports. |
| src-tauri/src/inference/errors.rs | Defines inference error types + helpers + tests. |
| src-tauri/src/inference/config.rs | Adds model config loading/interpolation + active-model resolution + tests. |
| src-tauri/src/commands/session.rs | Adds session list/load/delete/budget/cleanup IPC with richer payloads. |
| src-tauri/src/commands/python_env_startup.rs | Adds startup-time Python venv provisioning with logs. |
| src-tauri/src/commands/python_env.rs | Adds IPC-driven Python venv provisioning with progress events + tests. |
| src-tauri/src/commands/ollama.rs | Adds Ollama + llama.cpp status/model listing/pull IPC with progress events. |
| src-tauri/src/commands/model_download.rs | Adds streaming model download + SHA-256 verification + progress events. |
| src-tauri/src/commands/mod.rs | Registers commands module exports and placeholder greet command. |
| src-tauri/src/commands/hardware.rs | Adds hardware detection IPC with runtime/quantization recommendation. |
| src-tauri/src/commands/filesystem.rs | Adds stub filesystem browsing IPC and tilde expansion. |
| src-tauri/src/agent_core/types.rs | Adds core conversation/session/undo/audit + confirmation types + tests. |
| src-tauri/src/agent_core/tool_router.rs | Implements tool dispatch, confirmation flow, retries, audit/undo + tests. |
| src-tauri/src/agent_core/tool_prefilter.rs | Adds embedding-based tool RAG prefilter with error types + tests. |
| src-tauri/src/agent_core/tokens.rs | Adds token estimation + UTF-8 safe truncation + tests. |
| src-tauri/src/agent_core/response_analysis.rs | Adds heuristics for incomplete/deflection/completion responses + tests. |
| src-tauri/src/agent_core/plan_templates.rs | Adds template-based plan decomposition for common workflows + tests. |
| src-tauri/src/agent_core/plan_parser.rs | Adds bracket + JSON plan parsers + tests. |
| src-tauri/src/agent_core/permissions.rs | Adds tiered permission store with persistence + tests. |
| src-tauri/src/agent_core/mod.rs | Centralizes agent core module exports + re-exports. |
| src-tauri/src/agent_core/errors.rs | Adds agent-level error types + conversions. |
| src-tauri/mcp-servers.json | Adds empty MCP servers config placeholder. |
| src-tauri/entitlements.plist | Adds macOS entitlements required for spawning processes + networking. |
| src-tauri/capabilities/default.json | Adds default Tauri capabilities including shell/dialog permissions. |
| src-tauri/build.rs | Adds Tauri build script hook. |
| src-tauri/Cargo.toml | Defines Rust crate dependencies/features/profiles for Tauri backend. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (interval) { | ||
| clearInterval(interval); |
There was a problem hiding this comment.
stopConfigWatch references interval, but only configWatchInterval is in scope. This will throw at runtime and will also fail to stop the polling interval, leaving background work running. Use configWatchInterval in the conditional and when calling clearInterval(...).
| if (interval) { | |
| clearInterval(interval); | |
| if (configWatchInterval) { | |
| clearInterval(configWatchInterval); |
| * Implements debouncing to prevent duplicate sends. | ||
| */ | ||
|
|
||
| import { useCallback, useRef, useState } from "react"; |
There was a problem hiding this comment.
This file uses the React namespace (React.JSX.Element, React.KeyboardEvent, React.ChangeEvent) but doesn’t import React (type-only is fine). This typically fails TypeScript compilation under the modern JSX transform; either add import type React from "react"; or switch to importing the event/return types directly (e.g., KeyboardEvent, ChangeEvent, JSX.Element).
| import { useCallback, useRef, useState } from "react"; | |
| import { useCallback, useRef, useState } from "react"; | |
| import type React from "react"; |
| export function MessageInput({ | ||
| onSend, | ||
| disabled, | ||
| }: MessageInputProps): React.JSX.Element { |
There was a problem hiding this comment.
This file uses the React namespace (React.JSX.Element, React.KeyboardEvent, React.ChangeEvent) but doesn’t import React (type-only is fine). This typically fails TypeScript compilation under the modern JSX transform; either add import type React from "react"; or switch to importing the event/return types directly (e.g., KeyboardEvent, ChangeEvent, JSX.Element).
| } | ||
| }, [value, disabled, onSend]); | ||
|
|
||
| const handleKeyDown = (e: React.KeyboardEvent<HTMLTextAreaElement>): void => { |
There was a problem hiding this comment.
This file uses the React namespace (React.JSX.Element, React.KeyboardEvent, React.ChangeEvent) but doesn’t import React (type-only is fine). This typically fails TypeScript compilation under the modern JSX transform; either add import type React from "react"; or switch to importing the event/return types directly (e.g., KeyboardEvent, ChangeEvent, JSX.Element).
| } | ||
| }; | ||
|
|
||
| const handleInput = (e: React.ChangeEvent<HTMLTextAreaElement>): void => { |
There was a problem hiding this comment.
This file uses the React namespace (React.JSX.Element, React.KeyboardEvent, React.ChangeEvent) but doesn’t import React (type-only is fine). This typically fails TypeScript compilation under the modern JSX transform; either add import type React from "react"; or switch to importing the event/return types directly (e.g., KeyboardEvent, ChangeEvent, JSX.Element).
| onClick={clearConfigReloadNotification} | ||
| > | ||
| <span className="toast-icon">🔄</span> | ||
| <span className="toast-message">{configReloadNotification}</span> | ||
| <button className="toast-close" aria-label="Dismiss"> |
There was a problem hiding this comment.
The toast is clickable via a <div onClick=...> but isn’t keyboard accessible (no role/tabIndex/keyboard handlers). Also, the close <button> has no onClick, which makes its behavior reliant on event bubbling to the parent container. Prefer making the close button explicitly dismiss (with its own onClick, likely stopping propagation), and/or render the clickable surface as a <button> element or add appropriate role="button", tabIndex={0}, and Enter/Space key handling.
| onClick={clearConfigReloadNotification} | |
| > | |
| <span className="toast-icon">🔄</span> | |
| <span className="toast-message">{configReloadNotification}</span> | |
| <button className="toast-close" aria-label="Dismiss"> | |
| role="button" | |
| tabIndex={0} | |
| onClick={clearConfigReloadNotification} | |
| onKeyDown={(event) => { | |
| if (event.key === "Enter" || event.key === " ") { | |
| event.preventDefault(); | |
| clearConfigReloadNotification(); | |
| } | |
| }} | |
| > | |
| <span className="toast-icon">🔄</span> | |
| <span className="toast-message">{configReloadNotification}</span> | |
| <button | |
| className="toast-close" | |
| aria-label="Dismiss" | |
| type="button" | |
| onClick={(event) => { | |
| event.stopPropagation(); | |
| clearConfigReloadNotification(); | |
| }} | |
| > |
| } | ||
| } | ||
| } | ||
| Err("Python 3 not found. Install Python 3.11+ from https://python.org".to_string()) |
There was a problem hiding this comment.
The error message claims “Install Python 3.11+” but the implementation only checks that python3/python exists and responds to --version—it doesn’t validate the version. Either validate the version and keep the message, or adjust the message to avoid implying a minimum version that isn’t enforced.
| Err("Python 3 not found. Install Python 3.11+ from https://python.org".to_string()) | |
| Err("Python 3 not found. Install Python 3 from https://python.org".to_string()) |
| /// Check if the server process is still running. | ||
| pub async fn is_alive(&mut self) -> bool { | ||
| match self.process.try_wait() { | ||
| Ok(None) => true, // Still running | ||
| Ok(Some(_)) => false, // Exited | ||
| Err(_) => false, // Error checking — assume dead | ||
| } | ||
| } |
There was a problem hiding this comment.
is_alive is declared async but does not perform any .await or async I/O. Making it synchronous (or adding a comment explaining the async signature if required by a trait/interface elsewhere) will simplify call-sites and avoid suggesting it can yield.
| } | ||
| ], | ||
| "security": { | ||
| "csp": "default-src 'self'; connect-src 'self' http://localhost:* https://localhost:*; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob:; font-src 'self' data:" |
There was a problem hiding this comment.
The CSP includes style-src 'unsafe-inline' and allows connect-src to any localhost port. If this config is for production builds, consider tightening it (e.g., remove unsafe-inline by using hashed/nonced styles, and narrow connect-src to only the specific ports/hosts you need) to reduce XSS/exfiltration risk.
| "csp": "default-src 'self'; connect-src 'self' http://localhost:* https://localhost:*; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob:; font-src 'self' data:" | |
| "csp": "default-src 'self'; connect-src 'self' http://localhost:5173 https://localhost:5173; style-src 'self'; img-src 'self' data: blob:; font-src 'self' data:" |
| <true/> | ||
| <!-- Allow loading external dynamic libraries (Python packages, Node modules) --> | ||
| <key>com.apple.security.cs.disable-library-validation</key> | ||
| <true/> |
There was a problem hiding this comment.
These entitlements significantly weaken macOS code-signing/runtime protections (allow-unsigned-executable-memory, disable-library-validation). If they’re strictly required, it would help to (1) document the necessity and threat model, and (2) scope them as narrowly as possible (e.g., only for specific builds/targets) to reduce exposure in distributed binaries.
| <true/> | |
| <!-- Allow loading external dynamic libraries (Python packages, Node modules) --> | |
| <key>com.apple.security.cs.disable-library-validation</key> | |
| <true/> | |
| <false/> | |
| <!-- Allow loading external dynamic libraries (Python packages, Node modules) --> | |
| <key>com.apple.security.cs.disable-library-validation</key> | |
| <false/> |
Summary
Improves on PR #61 (error handling refactor) with better error messages and documentation:
Why this improves existing PRs
Testing