refactor: replace .expect() with proper error handling by vlordier · Pull Request #61 · Liquid4All/cookbook

vlordier · 2026-03-06T22:09:34Z

Summary

Replace .expect() calls with proper error handling in lib.rs
init_tracing(): log file open failure now prints error and returns
run(): database open failure now logs error and exits with code 1
run(): Tauri application run failure now logs error and exits with code 1
Add #[allow(dead_code)] for unused cache_dir() function
Add Default implementation for McpClient

Changes

47 files changed, ~14K insertions (includes full Tauri scaffolding from upstream)

Testing

All 426 tests pass
cargo clippy -- -D warnings passes

- 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%)

- init_tracing(): log file open failure now prints error and returns - run(): database open failure now logs error and exits with code 1 - run(): Tauri application run failure now logs error and exits with code 1 - Add #[allow(dead_code)] for cache_dir() - Add Default impl for McpClient

Copilot

Pull request overview

Refactors runtime error handling while introducing substantial new Tauri scaffolding and agent/MCP infrastructure for LocalCowork.

Changes:

Replaced panic-style .expect() paths with logging + controlled exits (per PR description).
Added frontend state management + UI components for settings/config watching and chat input.
Introduced MCP client + inference/agent-core modules (transport, lifecycle, tool routing, streaming parsing, permissions, etc.).

Reviewed changes

Copilot reviewed 48 out of 55 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/stores/settingsStore.ts	New Zustand store for settings data + config reload polling via Tauri invokes.
src/components/Chat/MessageInput.tsx	Message input UI with Enter-to-send and debounce behavior.
src/App.tsx	Root layout + config reload toast + settings watch lifecycle wiring.
src-tauri/tauri.conf.json	Tauri v2 configuration scaffold (window, CSP, bundling).
src-tauri/src/mcp_client/types.rs	MCP/JSON-RPC shared types + tests.
src-tauri/src/mcp_client/transport.rs	Stdio JSON-RPC transport + result extraction + tests.
src-tauri/src/mcp_client/mod.rs	MCP client module exports.
src-tauri/src/mcp_client/lifecycle.rs	Spawn/restart/shutdown handling for MCP servers.
src-tauri/src/mcp_client/errors.rs	Error types for MCP client.
src-tauri/src/mcp_client/discovery.rs	Auto-discovery of MCP servers + merge logic + tests.
src-tauri/src/mcp_client/client.rs	High-level MCP client API + lifecycle + tool execution + tests.
src-tauri/src/main.rs	Tauri app entrypoint calling `localcowork::run()`.
src-tauri/src/inference/types.rs	OpenAI-compatible request/response types + token-related helpers/tests.
src-tauri/src/inference/streaming.rs	SSE streaming parser + non-streaming fallback parsing + tests.
src-tauri/src/inference/mod.rs	Inference module exports.
src-tauri/src/inference/errors.rs	Inference error types + helpers/tests.
src-tauri/src/inference/config.rs	Models config loading/interpolation + resolution helpers + tests.
src-tauri/src/commands/session.rs	IPC for listing/loading/deleting sessions + context budget.
src-tauri/src/commands/python_env_startup.rs	Startup-time provisioning of Python MCP venvs.
src-tauri/src/commands/python_env.rs	IPC for provisioning Python MCP venvs + progress events + tests.
src-tauri/src/commands/ollama.rs	IPC for Ollama + llama.cpp health checks + model pull progress stream.
src-tauri/src/commands/model_download.rs	IPC for model downloads with progress + SHA-256 verification.
src-tauri/src/commands/mod.rs	Command module list + placeholder greet command.
src-tauri/src/commands/hardware.rs	IPC for hardware detection and runtime/quantization recommendations.
src-tauri/src/commands/filesystem.rs	Stub filesystem IPC for file browser UI.
src-tauri/src/agent_core/types.rs	Agent-core shared types (messages/sessions/audit/permissions/undo).
src-tauri/src/agent_core/tool_router.rs	Tool routing with confirmation + permissions + audit + undo + tests.
src-tauri/src/agent_core/tool_prefilter.rs	RAG embedding-based prefilter for tool selection + tests.
src-tauri/src/agent_core/tokens.rs	Token estimation + UTF-8 truncation + summarization helpers + tests.
src-tauri/src/agent_core/response_analysis.rs	Deflection/incompleteness detection + tests.
src-tauri/src/agent_core/plan_templates.rs	Template-based plan decomposition + tests.
src-tauri/src/agent_core/plan_parser.rs	Bracket + JSON plan parsers + tests.
src-tauri/src/agent_core/permissions.rs	Tiered permission grants + persistence + tests.
src-tauri/src/agent_core/mod.rs	Agent core module exports.
src-tauri/src/agent_core/errors.rs	Agent core error types + conversions.
src-tauri/mcp-servers.json	Empty MCP servers config scaffold.
src-tauri/entitlements.plist	macOS entitlements for process spawning/library validation/network/files.
src-tauri/capabilities/default.json	Default Tauri capabilities/permissions scaffold.
src-tauri/build.rs	Tauri build script.
src-tauri/Cargo.toml	Rust/Tauri dependencies and release profile settings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T22:12:54Z

+    if (interval) {
+      clearInterval(interval);


interval is not defined here; this will fail to compile (and would be a runtime ReferenceError if it did). Use configWatchInterval for the null-check and clearInterval call.

Suggested change

if (interval) {

clearInterval(interval);

if (configWatchInterval) {

clearInterval(configWatchInterval);

Copilot · 2026-03-06T22:12:54Z

+        // fs was overridden
+        assert_eq!(merged["fs"].command, "node");
+        // ocr was NOT overridden — preserved from discovery
+        assert_eq!(merged["ocr"].command, "npx");


This test is not platform-agnostic: ts_config() uses default_npx_command(), which is npx.cmd on Windows. Update the expectation to use default_npx_command() (or branch on cfg!(windows)) so the test passes cross-platform.

Suggested change

assert_eq!(merged["ocr"].command, "npx");

let expected_ocr_command = ts_config("ocr").command;

assert_eq!(merged["ocr"].command, expected_ocr_command);

Copilot · 2026-03-06T22:12:55Z

+        loop {
+            line_buf.clear();
+            let bytes_read = reader
+                .read_line(&mut line_buf)
+                .await
+                .map_err(|e| McpError::TransportError {
+                    server: self.server_name.clone(),
+                    reason: format!("failed to read from stdout: {e}"),
+                })?;
+
+            if bytes_read == 0 {
+                return Err(McpError::TransportError {
+                    server: self.server_name.clone(),
+                    reason: "server stdout closed (process may have exited)".into(),
+                });
+            }
+
+            let trimmed = line_buf.trim();
+            if trimmed.is_empty() {
+                continue;
+            }
+
+            // Try to parse as JSON-RPC response
+            match serde_json::from_str::<JsonRpcResponse>(trimmed) {
+                Ok(resp) if resp.id == id => return Ok(resp),
+                Ok(_) => {
+                    // Response for a different request ID — skip
+                    // This shouldn't happen in our single-threaded protocol,
+                    // but handle gracefully.
+                    continue;
+                }
+                Err(_) => {
+                    // Not a JSON-RPC response — could be server log output.
+                    // Skip and keep reading.
+                    continue;
+                }
+            }
+        }


If multiple request() calls are ever in-flight concurrently on the same StdioTransport, this loop can permanently drop responses: a response for a different id is discarded, so the corresponding caller will hang until EOF/timeout. Tangible fix options: (1) enforce single in-flight request per transport by adding a dedicated mutex around the entire request (write+read) so only one id can exist at a time, or (2) spawn a dedicated reader task that demultiplexes responses into a per-id map/channel and never discards valid JSON-RPC responses.

Copilot · 2026-03-06T22:12:55Z

+        // Write to temp file, then rename for atomicity
+        let tmp_path = self.persist_path.with_extension("json.tmp");
+        if let Err(e) = std::fs::write(&tmp_path, &content) {
+            tracing::error!(error = %e, "failed to write permissions temp file");
+            return;
+        }
+        if let Err(e) = std::fs::rename(&tmp_path, &self.persist_path) {
+            tracing::error!(error = %e, "failed to rename permissions file");
+            return;
+        }


std::fs::rename is not a reliable atomic replace on Windows when the destination file already exists (it commonly fails rather than replacing). This can break updating persistent permissions after the first save. Use a Windows-safe atomic write strategy (e.g., remove the destination first on Windows, or use a temp file + replace/persist mechanism such as tempfile's NamedTempFile::persist, or a cross-platform atomic-write crate).

Copilot · 2026-03-06T22:12:55Z

+}
+
+/// Cosine similarity between two L2-normalized vectors (= dot product).
+fn cosine_similarity(a: &[f32], b: &[f32]) -> f32 {


Dimension mismatches are silently ignored here because zip() truncates to the shorter length, yielding incorrect similarity scores. Since ToolPreFilterError::DimensionMismatch already exists, validate a.len() == b.len() (and/or validate query/tool dims once in filter()/build()) and return a structured error when they differ.

Suggested change

fn cosine_similarity(a: &[f32], b: &[f32]) -> f32 {

fn cosine_similarity(a: &[f32], b: &[f32]) -> f32 {

if a.len() != b.len() {

panic!(

"cosine_similarity dimension mismatch: expected {} elements, got {}",

a.len(),

b.len()

);

}

Copilot · 2026-03-06T22:12:55Z

+use super::python_env::{find_system_python, is_venv_ready, pip_executable, venv_bin_dir};
+
+// ─── Discovery ──────────────────────────────────────────────────────────────
+
+/// Find all Python MCP server directories (those with `pyproject.toml`).
+fn discover_python_servers(project_root: &Path) -> Vec<(String, PathBuf)> {
+    let mcp_dir = project_root.join("mcp-servers");
+    let mut servers = Vec::new();
+
+    let entries = match std::fs::read_dir(&mcp_dir) {
+        Ok(e) => e,
+        Err(_) => return servers,
+    };
+
+    for entry in entries.flatten() {
+        let path = entry.path();
+        if !path.is_dir() {
+            continue;
+        }
+
+        let name = match path.file_name().and_then(|n| n.to_str()) {
+            Some(n) => n.to_string(),
+            None => continue,
+        };
+
+        if name.starts_with('_') || name.starts_with('.') {
+            continue;
+        }
+
+        if path.join("pyproject.toml").exists() {
+            servers.push((name, path));
+        }
+    }
+
+    servers
+}
+


This discovery logic is duplicated in src-tauri/src/commands/python_env.rs (discover_python_servers). Consider extracting a shared helper (e.g., a small internal module or a function in python_env reused by python_env_startup) to avoid divergence and ensure consistent filtering behavior.

Suggested change

use super::python_env::{find_system_python, is_venv_ready, pip_executable, venv_bin_dir};

// ─── Discovery ──────────────────────────────────────────────────────────────

/// Find all Python MCP server directories (those with `pyproject.toml`).

fn discover_python_servers(project_root: &Path) -> Vec<(String, PathBuf)> {

let mcp_dir = project_root.join("mcp-servers");

let mut servers = Vec::new();

let entries = match std::fs::read_dir(&mcp_dir) {

Ok(e) => e,

Err(_) => return servers,

};

for entry in entries.flatten() {

let path = entry.path();

if !path.is_dir() {

continue;

}

let name = match path.file_name().and_then(|n| n.to_str()) {

Some(n) => n.to_string(),

None => continue,

};

if name.starts_with('_') || name.starts_with('.') {

continue;

}

if path.join("pyproject.toml").exists() {

servers.push((name, path));

}

}

servers

}

use super::python_env::{

discover_python_servers, find_system_python, is_venv_ready, pip_executable, venv_bin_dir,

};

Copilot · 2026-03-06T22:12:56Z

+      {configReloadNotification && (
+        <div
+          className="config-reload-toast"
+          onClick={clearConfigReloadNotification}
+        >
+          <span className="toast-icon">🔄</span>
+          <span className="toast-message">{configReloadNotification}</span>
+          <button className="toast-close" aria-label="Dismiss">
+            ×
+          </button>
+        </div>
+      )}


The toast is dismissible via onClick on a non-interactive <div>, which is not keyboard accessible by default. Prefer making the dismiss action a real <button> (or add role=\"button\", tabIndex={0}, and Enter/Space key handling). Also consider adding type=\"button\" and wiring the close button’s onClick explicitly (optionally stopPropagation) so dismissal doesn’t depend on event bubbling.

Copilot · 2026-03-06T22:12:56Z

+        <button
+          className={`send-button ${isDebouncing ? "debouncing" : ""}`}
+          onClick={handleSend}
+          disabled={isLoading || !value.trim()}
+          aria-label={isDebouncing ? "Please wait..." : "Send message"}
+        >


This <button> has no explicit type. If MessageInput is ever rendered inside a <form>, the default type=\"submit\" can cause unintended form submission. Set type=\"button\" for a pure action button.

- Add comment explaining why eprintln! is used (tracing not yet initialized) - Update docstring to mention early return on error

vlordier added 5 commits March 6, 2026 21:46

test: add unit tests for PR features

8799d20

- 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%)

Copilot AI review requested due to automatic review settings March 6, 2026 22:09

Copilot AI reviewed Mar 6, 2026

View reviewed changes

chore: clarify init_tracing error handling in docs

0495e3d

- Add comment explaining why eprintln! is used (tracing not yet initialized) - Update docstring to mention early return on error

vlordier mentioned this pull request Mar 6, 2026

chore: improve error messages and documentation #66

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: replace .expect() with proper error handling#61

refactor: replace .expect() with proper error handling#61
vlordier wants to merge 6 commits intoLiquid4All:mainfrom
vlordier:refactor/error-handling

vlordier commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	assert_eq!(merged["ocr"].command, "npx");
	let expected_ocr_command = ts_config("ocr").command;
	assert_eq!(merged["ocr"].command, expected_ocr_command);

-fn cosine_similarity(a: &[f32], b: &[f32]) -> f32 {
+fn cosine_similarity(a: &[f32], b: &[f32]) -> f32 {
+    if a.len() != b.len() {
+        panic!(
+            "cosine_similarity dimension mismatch: expected {} elements, got {}",
+            a.len(),
+            b.len()
+        );
+    }

Conversation

vlordier commented Mar 6, 2026

Summary

Changes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants