chore(clippy): bring cargo clippy to zero warnings and enforce -D warnings in CI (#205)

* chore(clippy): relocate cfg(test)-gated imports into test modules

EnvPlaceholder (mcp_registry.rs) and DEFAULT_MCP_SERVER_PORT (mcp_server.rs)
were imported at module level but only consumed inside #[cfg(test)] blocks.
This made them look unused to clippy's lib pass and invited a cargo-fix
regression that would strip them and break the test build. Moving the imports
into the test module where they are used eliminates the warning without
risking the test build.

Co-Authored-By: Claude <noreply@anthropic.com>

* chore(clippy): prefix unused variables with underscore

Resolves 'unused variable' warnings across 6 files by adding the _ prefix
idiom. In claude_settings.rs the mut binding was also dropped because the
variable was never mutated after the read. All sites are in test code or
Tauri command handlers where the variable captures a value kept for lifetime
purposes (app_handle.clone()) but not referenced directly.

Co-Authored-By: Claude <noreply@anthropic.com>

* chore(clippy): replace useless vec! with arrays

Sites in test code that collect JSONL lines or MCP configs into a temporary
collection only to call .join() or pass as a slice. Array literal is cheaper
(no heap allocation) and expresses intent more directly. 26 sites in
session_explorer.rs tests, 2 in config_writer.rs tests — all previously
flagged by 'clippy::useless_vec'.

Co-Authored-By: Claude <noreply@anthropic.com>

* chore(clippy): resolve dead-code warnings via targeted allows

66 dead-code warnings broken into four sub-patterns, each with its
own appropriate fix:

1. Test-helper pub(crate) fns (45 sites across 9 files): #[cfg_attr(not(test),
   allow(dead_code))]. These are #[cfg(test)]-consumed helpers exposed as
   pub(crate) so the test module can bypass Tauri State<> wrappers. The attr
   suppresses the warning for non-test builds while preserving genuine
   dead-code detection in test builds.

2. Vestigial typed-parser vendor hierarchy: 4 enums (CodexMcp, CopilotMcp,
   CursorMcp, GeminiMcp) + 8 Stdio/Http variant structs. These were
   scaffolded for a typed intermediate-representation parse path that was
   never wired up; current code uses ParsedXxxMcp normalization directly.
   Annotated with #[allow(dead_code)] + a comment noting the design intent so
   they remain available for future typed-parse work.

3. Serde-deser false positives (DevcontainerConfig, ProfileItem,
   SpinnerVerbConfig, 3 fields on RawRecord/RawMessage/ToolManagerServer).
   Clippy's 'never constructed' lint does not see serde's
   Deserialize-generated constructors. Standard #[allow(dead_code)] idiom.

4. One genuine deletion: Database::from_connection in db/schema.rs. Already
   #[cfg(test)]-gated with zero callers; sibling Database::in_memory() is the
   actual entry point used throughout tests.

Dead-code warning count: 66 -> 0. Total clippy warnings: 134 -> 68.

Co-Authored-By: Claude <noreply@anthropic.com>

* chore(clippy): apply mechanical autofix suggestions

Broad pass applying clippy's machine-applicable suggestions now that the
cfg(test)-gated-import and unused-variable landmines have been resolved.
Covers:

  - manual Option::map rewrites (several sites in mcp_client.rs, skills.rs,
    session_explorer.rs)
  - assert_eq!(x, true/false) -> assert!(x) / assert!(!x)
  - useless format! dropped
  - redundant closures replaced with the function itself
  - Iterator::last on DoubleEndedIterator -> .next_back()
  - map_or simplification
  - Option::map().flatten() -> and_then
  - map().values() iteration cleanup
  - assorted rustfmt normalization introduced by the rewrites

Additionally: replaced a tautological bool assertion in debug_logger.rs
(assert!(enabled == true || enabled == false)) with 'let _ = is_debug_enabled();'
matching the adjacent test's 'let _ = path;' pattern — the assertion was
already always-true before clippy rewrote it, and the newer nonminimal_bool
lint now flags the explicit form as an error under -D warnings.

Clippy warning count: 68 -> 33.

Co-Authored-By: Claude <noreply@anthropic.com>

* chore(clippy): accept &Path instead of &PathBuf in helper signatures

Clippy::ptr_arg prefers &Path over &PathBuf because the former is a slice
type and avoids a pointless heap indirection. 10 sites across 3 files:
  - src/services/debug_logger.rs (enable_debug_logging, get_logs_dir,
    get_debug_flag_path)
  - src/utils/paths.rs (project_mcp_file, project_settings_file)
  - src/utils/opencode_paths.rs (5 project_opencode_* helpers)
Call sites already pass &PathBuf, which auto-dereferences to &Path — no
caller changes required.

Co-Authored-By: Claude <noreply@anthropic.com>

* chore(clippy): allow type_complexity on DB-row-tuple helpers

7 command helpers use multi-field tuples (7-10 fields) for rusqlite row
extraction. These tuples ARE the domain shape — introducing type aliases
would just duplicate the tuple next to the fn without adding meaning,
and the existing file-local McpTuple aliases in services/*_config.rs
follow a different shape (writer tuples, not reader tuples).

Annotating with #[allow(clippy::type_complexity)] on the containing fn
matches the established pattern for 'this IS the data shape' cases.

Co-Authored-By: Claude <noreply@anthropic.com>

* chore(clippy): resolve long-tail warnings to reach zero

Final cleanup batch:

- opencode_paths.rs: #[cfg(test)] use std::path::Path; — Path is only needed
  by the cfg(test)-gated helpers that now take &Path.
- debug_logger.rs: &PathBuf -> &Path on 3 more helpers (persist_debug_enabled,
  is_debug_persisted, init_from_persisted). Missed in the first PathBuf pass.
- db/models.rs: #[cfg_attr(not(test), allow(dead_code))] on is_powerline and
  is_powerline_round — test-only consumers.
- docker/client.rs: #[allow(dead_code)] on connect_with_params (private
  helper used via ping_host) and ping_host (public API held for future use).
- docker/devcontainer.rs: #[allow(dead_code)] on the impl block — several
  methods are defined for the DevcontainerConfig API shape but not all are
  consumed today.
- commands/mod.rs: #[allow(clippy::module_inception)] on `pub mod commands`
  — the nested name mirrors the Claude Code commands domain concept;
  renaming would cascade through imports.
- mcp_gateway/tools.rs: #[allow(clippy::manual_async_fn)] on
  impl ServerHandler — rmcp's trait uses impl Future return shape; async fn
  in traits isn't usable here without widening the bound.
- commands/scanner.rs: removed tautological assert!(true) in the
  compilation-smoke-test (comment documents intent instead).
- services/scanner.rs:
  - #[allow(clippy::too_many_arguments)] on get_or_create_mcp (9/7)
  - strip_prefix("---") rewrite in parse_frontmatter
  - assert!(!fm.contains_key("empty_key")) replaces .get().is_none()
  - #[allow(non_snake_case)] on test_parse_skill_file_with_allowedTools_camelCase
    where the camelCase mirrors the schema field under test.
- services/mcp_registry.rs: filter_map always-Some -> map.

Clippy warning count: 17 -> 0. Tests: 2021 passing. Ready to flip CI gate.

Co-Authored-By: Claude <noreply@anthropic.com>

* ci(clippy): enforce -D warnings on lib + tests

Flips the Clippy job to block CI on warnings. Two tightenings:

  - Drop 'continue-on-error: true'. The comment said 'initially' — eight
    commits of cleanup later, the baseline is zero warnings, so the gate
    becomes meaningful.
  - Add '--lib --tests' to the clippy invocation so the ~120 warnings that
    lived exclusively in the test target are covered too. Without this the
    prior gate (if enforced) would have allowed test-only lint debt to
    accumulate invisibly.

All other CI jobs untouched. No policy change beyond Clippy enforcement.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(clippy): address rust 1.95 warnings

---------

Co-authored-by: Scot Campbell <prefrontalsys@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>

Author: prefrontalsys

.github/workflows/rust-tests.yml

@@ -92,8 +92,7 @@ jobs:
 
       - name: Run Clippy
         working-directory: src-tauri
-        run: cargo clippy --all-features -- -D warnings
-        continue-on-error: true  # Don't fail the build on clippy warnings initially
+        run: cargo clippy --all-targets --all-features -- -D warnings
 
   fmt:
     name: Format

src-tauri/src/commands/commands.rs

@@ -329,13 +329,11 @@ pub fn get_global_commands(
     db: State<'_, Arc<Mutex<Database>>>,
 ) -> Result<Vec<GlobalCommand>, String> {
     let db = db.lock().map_err(|e| e.to_string())?;
-    let query = format!(
-        "SELECT gc.id, gc.command_id, gc.is_enabled,
+    let query = "SELECT gc.id, gc.command_id, gc.is_enabled,
                 c.id, c.name, c.description, c.content, c.allowed_tools, c.argument_hint, c.model, c.tags, c.source, c.source_path, c.is_favorite, c.created_at, c.updated_at
          FROM global_commands gc
          JOIN commands c ON gc.command_id = c.id
-         ORDER BY c.name"
-    );
+         ORDER BY c.name".to_string();
     let mut stmt = db.conn().prepare(&query).map_err(|e| e.to_string())?;
 
     let commands = stmt
@@ -644,13 +642,11 @@ pub fn toggle_project_command(
         .map_err(|e| e.to_string())?;
 
     // Get the command and project path
-    let query = format!(
-        "SELECT c.id, c.name, c.description, c.content, c.allowed_tools, c.argument_hint, c.model, c.tags, c.source, c.source_path, c.is_favorite, c.created_at, c.updated_at, p.path
+    let query = "SELECT c.id, c.name, c.description, c.content, c.allowed_tools, c.argument_hint, c.model, c.tags, c.source, c.source_path, c.is_favorite, c.created_at, c.updated_at, p.path
          FROM project_commands pc
          JOIN commands c ON pc.command_id = c.id
          JOIN projects p ON pc.project_id = p.id
-         WHERE pc.id = ?"
-    );
+         WHERE pc.id = ?".to_string();
     let mut stmt = db_guard.conn().prepare(&query).map_err(|e| e.to_string())?;
 
     let (command, project_path): (Command, String) = stmt
@@ -706,14 +702,12 @@ pub fn get_project_commands(
     project_id: i64,
 ) -> Result<Vec<ProjectCommand>, String> {
     let db = db.lock().map_err(|e| e.to_string())?;
-    let query = format!(
-        "SELECT pc.id, pc.command_id, pc.is_enabled,
+    let query = "SELECT pc.id, pc.command_id, pc.is_enabled,
                 c.id, c.name, c.description, c.content, c.allowed_tools, c.argument_hint, c.model, c.tags, c.source, c.source_path, c.is_favorite, c.created_at, c.updated_at
          FROM project_commands pc
          JOIN commands c ON pc.command_id = c.id
          WHERE pc.project_id = ?
-         ORDER BY c.name"
-    );
+         ORDER BY c.name".to_string();
     let mut stmt = db.conn().prepare(&query).map_err(|e| e.to_string())?;
 
     let commands = stmt

src-tauri/src/commands/config.rs

@@ -70,6 +70,7 @@ pub fn sync_global_config(db: State<'_, Arc<Mutex<Database>>>) -> Result<(), Str
 }
 
 /// Sync global config from database to disk (reusable helper without Tauri State)
+#[allow(clippy::type_complexity)]
 pub(crate) fn sync_global_config_from_db(db: &Database) -> Result<(), String> {
     use crate::commands::settings::get_enabled_editors_from_db;
     use crate::services::{

src-tauri/src/commands/docker_hosts.rs

@@ -14,7 +14,7 @@ pub fn get_all_docker_hosts(
 #[tauri::command]
 pub fn create_docker_host(
     db: State<'_, Arc<Mutex<Database>>>,
-    request: CreateDockerHostRequest,
+    _request: CreateDockerHostRequest,
 ) -> Result<DockerHost, String> {
     let _db = db.lock().map_err(|e| e.to_string())?;
     Err("Docker host feature not yet implemented".to_string())
@@ -23,20 +23,20 @@ pub fn create_docker_host(
 #[tauri::command]
 pub fn update_docker_host(
     db: State<'_, Arc<Mutex<Database>>>,
-    id: i64,
-    request: CreateDockerHostRequest,
+    _id: i64,
+    _request: CreateDockerHostRequest,
 ) -> Result<DockerHost, String> {
     let _db = db.lock().map_err(|e| e.to_string())?;
     Err("Docker host feature not yet implemented".to_string())
 }
 
 #[tauri::command]
-pub fn delete_docker_host(db: State<'_, Arc<Mutex<Database>>>, id: i64) -> Result<(), String> {
+pub fn delete_docker_host(db: State<'_, Arc<Mutex<Database>>>, _id: i64) -> Result<(), String> {
     let _db = db.lock().map_err(|e| e.to_string())?;
     Err("Docker host feature not yet implemented".to_string())
 }
 
 #[tauri::command]
-pub fn test_docker_host(id: i64) -> Result<bool, String> {
+pub fn test_docker_host(_id: i64) -> Result<bool, String> {
     Err("Docker host feature not yet implemented".to_string())
 }

src-tauri/src/commands/hooks.rs

@@ -910,6 +910,7 @@ pub fn duplicate_hook(
 // ============================================================================
 
 /// Create a hook in the database (no file sync)
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn create_hook_in_db(db: &Database, hook: &CreateHookRequest) -> Result<Hook, String> {
     let tags_json = hook
         .tags
@@ -939,6 +940,7 @@ pub(crate) fn create_hook_in_db(db: &Database, hook: &CreateHookRequest) -> Resu
 }
 
 /// Get a hook by ID from the database
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn get_hook_by_id(db: &Database, id: i64) -> Result<Hook, String> {
     let mut stmt = db
         .conn()
@@ -971,6 +973,7 @@ pub(crate) fn get_all_hooks_from_db(db: &Database) -> Result<Vec<Hook>, String>
 }
 
 /// Update a hook in the database (no file sync)
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn update_hook_in_db(
     db: &Database,
     id: i64,
@@ -1020,6 +1023,7 @@ pub(crate) fn update_hook_in_db(
 }
 
 /// Delete a hook from the database (no file sync)
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn delete_hook_from_db(db: &Database, id: i64) -> Result<(), String> {
     db.conn()
         .execute("DELETE FROM hooks WHERE id = ?", [id])
@@ -1028,6 +1032,7 @@ pub(crate) fn delete_hook_from_db(db: &Database, id: i64) -> Result<(), String>
 }
 
 /// Add a hook to global hooks in the database (no file sync)
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn add_global_hook_in_db(db: &Database, hook_id: i64) -> Result<(), String> {
     db.conn()
         .execute(
@@ -1039,6 +1044,7 @@ pub(crate) fn add_global_hook_in_db(db: &Database, hook_id: i64) -> Result<(), S
 }
 
 /// Get all global hooks from the database
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn get_global_hooks_from_db(db: &Database) -> Result<Vec<GlobalHook>, String> {
     let mut stmt = db
         .conn()
@@ -1068,6 +1074,7 @@ pub(crate) fn get_global_hooks_from_db(db: &Database) -> Result<Vec<GlobalHook>,
 }
 
 /// Toggle a global hook in the database (no file sync)
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn toggle_global_hook_in_db(
     db: &Database,
     id: i64,
@@ -1083,6 +1090,7 @@ pub(crate) fn toggle_global_hook_in_db(
 }
 
 /// Remove a global hook from the database (no file sync)
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn remove_global_hook_from_db(db: &Database, hook_id: i64) -> Result<(), String> {
     db.conn()
         .execute("DELETE FROM global_hooks WHERE hook_id = ?", [hook_id])

src-tauri/src/commands/mcp.rs

@@ -235,6 +235,7 @@ pub(crate) fn delete_mcp_impl(db: &Database, id: i64) -> Result<(), String> {
 }
 
 /// Duplicate an MCP in the database
+#[allow(clippy::type_complexity)]
 pub(crate) fn duplicate_mcp_impl(db: &Database, id: i64) -> Result<Mcp, String> {
     // Get original
     let mut stmt = db
@@ -319,18 +320,22 @@ pub(crate) fn generate_duplicate_name(name: &str) -> String {
 }
 
 // Convenience aliases (promoted from #[cfg(test)] for cross-module reuse)
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn create_mcp_in_db(db: &Database, mcp: &CreateMcpRequest) -> Result<Mcp, String> {
     create_mcp_impl(db, mcp)
 }
 
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn get_mcp_by_id(db: &Database, id: i64) -> Result<Mcp, String> {
     get_mcp_impl(db, id)
 }
 
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn get_all_mcps_from_db(db: &Database) -> Result<Vec<Mcp>, String> {
     get_all_mcps_impl(db)
 }
 
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn update_mcp_in_db(
     db: &Database,
     id: i64,
@@ -339,10 +344,12 @@ pub(crate) fn update_mcp_in_db(
     update_mcp_impl(db, id, mcp)
 }
 
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn delete_mcp_from_db(db: &Database, id: i64) -> Result<(), String> {
     delete_mcp_impl(db, id)
 }
 
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn toggle_global_mcp_in_db(db: &Database, id: i64, enabled: bool) -> Result<(), String> {
     toggle_global_mcp_impl(db, id, enabled)
 }
@@ -708,7 +715,7 @@ mod tests {
         let db = Database::in_memory().unwrap();
         let req = sample_stdio_mcp();
         let created = create_mcp_in_db(&db, &req).unwrap();
-        let original_updated = created.updated_at.clone();
+        let _original_updated = created.updated_at.clone();
 
         // Small delay to ensure timestamp changes
         std::thread::sleep(std::time::Duration::from_millis(10));

src-tauri/src/commands/mcp_registry.rs

@@ -1,5 +1,5 @@
 use crate::db::Database;
-use crate::services::mcp_registry::{EnvPlaceholder, RegistryClient, RegistryMcpEntry};
+use crate::services::mcp_registry::{RegistryClient, RegistryMcpEntry};
 use rusqlite::params;
 use serde::{Deserialize, Serialize};
 use std::sync::{Arc, Mutex};
@@ -94,18 +94,15 @@ pub fn import_mcp_from_registry_in_db(
     let args_json = entry
         .args
         .as_ref()
-        .map(|a| serde_json::to_string(a).ok())
-        .flatten();
+        .and_then(|a| serde_json::to_string(a).ok());
     let headers_json = entry
         .headers
         .as_ref()
-        .map(|h| serde_json::to_string(h).ok())
-        .flatten();
+        .and_then(|h| serde_json::to_string(h).ok());
     let env_json = entry
         .env
         .as_ref()
-        .map(|e| serde_json::to_string(e).ok())
-        .flatten();
+        .and_then(|e| serde_json::to_string(e).ok());
 
     db.conn()
         .execute(
@@ -166,6 +163,7 @@ pub fn get_registry_mcp_by_id(db: &Database, id: i64) -> Result<RegistryMcpEntry
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::services::mcp_registry::EnvPlaceholder;
     use std::collections::HashMap;
 
     // =========================================================================

src-tauri/src/commands/mcp_server.rs

@@ -3,9 +3,7 @@
 //! These commands allow the frontend to start/stop and configure the MCP server.
 
 use crate::db::Database;
-use crate::mcp_server::server::{
-    generate_self_mcp_entry, McpServerConfig, McpServerStatus, DEFAULT_MCP_SERVER_PORT,
-};
+use crate::mcp_server::server::{generate_self_mcp_entry, McpServerConfig, McpServerStatus};
 use crate::mcp_server::McpServerState;
 use log::info;
 use serde_json::Value;
@@ -148,6 +146,7 @@ pub fn is_self_mcp_in_library(db: State<'_, Arc<Mutex<Database>>>) -> Result<boo
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::mcp_server::server::DEFAULT_MCP_SERVER_PORT;
 
     #[test]
     fn test_mcp_server_config_serde() {

src-tauri/src/commands/mcp_session.rs

@@ -25,6 +25,7 @@ pub struct McpSessionData {
 }
 
 /// Extract MCP session data from the database (no Tauri State dependency)
+#[allow(clippy::type_complexity)]
 pub(crate) fn get_mcp_session_data_from_db(
     db: &Database,
     mcp_id: i64,
@@ -84,6 +85,7 @@ pub(crate) fn get_mcp_session_data_from_db(
 }
 
 /// Validate MCP session data before starting a session
+#[cfg_attr(not(test), allow(dead_code))]
 pub(crate) fn validate_mcp_session_data(data: &McpSessionData) -> Result<(), String> {
     if data.source == "system" {
         if data.url.is_none() {

src-tauri/src/commands/mcp_test.rs

@@ -10,6 +10,7 @@ use std::sync::{Arc, Mutex};
 use tauri::State;
 
 /// Extract MCP test data including source field from the database (no Tauri State dependency)
+#[allow(clippy::type_complexity)]
 pub fn get_mcp_test_data_with_source_from_db(
     db: &Database,
     mcp_id: i64,
@@ -163,6 +164,8 @@ pub fn test_mcp_config(
 // ============================================================================
 
 /// Extract MCP data from database for testing
+#[cfg_attr(not(test), allow(dead_code))]
+#[allow(clippy::type_complexity)]
 pub fn get_mcp_test_data_from_db(
     db: &Database,
     mcp_id: i64,
@@ -217,6 +220,7 @@ pub fn get_mcp_test_data_from_db(
 }
 
 /// Validate MCP config before testing
+#[cfg_attr(not(test), allow(dead_code))]
 pub fn validate_mcp_config(
     mcp_type: &str,
     command: Option<&str>,