diff --git a/DEV.md b/DEV.md index 63f3e2b4..f7399c41 100644 --- a/DEV.md +++ b/DEV.md @@ -320,7 +320,7 @@ Source: [`crates/konnect-core/src/observability.rs`](crates/konnect-core/src/obs ## Tool Routing (Starter Kit + On-Demand Loading) -The server does NOT expose all 224 tools (231 total with the 7 meta-tools) in `tools/list` by default — that would cost ~23K tokens of context on every listing. Instead: +The server does NOT expose all 225 tools (232 total with the 7 meta-tools) in `tools/list` by default — that would cost ~23K tokens of context on every listing. Instead: - **Startup**: only `STARTER_KIT` toolsets are pre-loaded (see `router/registry.rs::STARTER_KIT`). Currently: `project`, `config`. Combined with the 7 meta-tools, baseline `tools/list` is 21 tools ≈ 2K tokens. - **On demand**: the LLM reads `list_toolboxes` → calls `load_toolset(name)` to expose a toolset's tools in subsequent `tools/list` responses. `unload_toolset(name)` prunes them when the task shifts. @@ -395,9 +395,9 @@ convention for other `kicad-cli`-calling code. ## Current Stats -- **21 toolsets, 224 tools** + 7 meta-tools (4 routing + 2 observability + 1 runtime diagnostic — see `tool-directory.md`) +- **21 toolsets, 225 tools** + 7 meta-tools (4 routing + 2 observability + 1 runtime diagnostic — see `tool-directory.md`) - Baseline `tools/list`: 21 tools / ~2K tokens (starter kit + meta-tools) -- Full-catalog `tools/list` (all loaded): 231 tools (224 registered + 7 meta) / ~25K tokens +- Full-catalog `tools/list` (all loaded): 232 tools (225 registered + 7 meta) / ~25K tokens - **0 IPC stubs** (all protobuf methods implemented) - **0 unimplemented tools** - **Specctra DSN/SES are PCB-editor operations**, not `kicad-cli` commands. diff --git a/README.md b/README.md index 448290fa..89d0dad0 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ Rust binary — that lets Claude and other AI assistants design schematics and PCBs through the [Model Context Protocol](https://modelcontextprotocol.io) (MCP). -**224 tools across 21 on-demand toolsets.** Schematic capture, PCB layout and +**225 tools across 21 on-demand toolsets.** Schematic capture, PCB layout and routing, ERC/DRC, design-review audits, JLCPCB part search, reference circuits, and a full manufacturing export pipeline — with bundled skills and agents that teach Claude KiCAD conventions out of the box. @@ -70,7 +70,7 @@ through its own S-expression engine with atomic writes (write, fsync, rename), U preservation, and round-trip tests — no third-party schematic library with known gaps, no text-manipulation workarounds. -**Context economy is a feature.** Exposing all 224 tools to an LLM costs roughly 23K +**Context economy is a feature.** Exposing all 225 tools to an LLM costs roughly 23K tokens of context on every listing. Konnect's router loads a starter kit (~2K tokens) and lets the model pull in toolsets on demand — plus built-in observability (`get_recent_calls`, `server_stats`, JSONL call logs) so the model can diagnose its diff --git a/crates/konnect-core/src/mcp/error.rs b/crates/konnect-core/src/mcp/error.rs index 72f138cc..fe8a6ec4 100644 --- a/crates/konnect-core/src/mcp/error.rs +++ b/crates/konnect-core/src/mcp/error.rs @@ -87,6 +87,14 @@ pub enum ToolErrorKind { capability: String, kicad_version: Option, }, + /// KiCad accepted a semantic mutation, but a fresh observation did not + /// prove the exact requested post-operation state. + ReadbackMismatch { + operation: String, + requested_kiids: Vec, + before_kiids: Vec, + after_kiids: Vec, + }, /// A board was live earlier in this server process, but IPC is now gone; /// its saved file may be stale relative to lost editor state. UnsafeFileFallback { path: String, reason: String }, @@ -117,6 +125,7 @@ impl ToolErrorKind { Self::StaleTarget { .. } => "stale_target", Self::EditorUnavailable { .. } => "editor_unavailable", Self::UnsupportedCapability { .. } => "unsupported_capability", + Self::ReadbackMismatch { .. } => "readback_mismatch", Self::UnsafeFileFallback { .. } => "unsafe_file_fallback", Self::AmbiguousOpenBoard { .. } => "ambiguous_open_board", Self::HandlerError { .. } => "handler_error", @@ -270,6 +279,12 @@ mod tests { capability: "activate_sheet".into(), kicad_version: Some("10.0.5".into()), }, + ToolErrorKind::ReadbackMismatch { + operation: "add".into(), + requested_kiids: vec!["b".into()], + before_kiids: vec!["a".into()], + after_kiids: vec!["a".into()], + }, ToolErrorKind::UnsafeFileFallback { path: "p".into(), reason: "r".into(), diff --git a/crates/konnect-core/src/router/registry.rs b/crates/konnect-core/src/router/registry.rs index c20f9166..c227def1 100644 --- a/crates/konnect-core/src/router/registry.rs +++ b/crates/konnect-core/src/router/registry.rs @@ -28,7 +28,7 @@ pub static ALL_TOOLSETS: &[ToolsetMeta] = &[ name: "editor_navigation", description: "Observe and semantically navigate exact KiCad editor, document, sheet, selection, and cross-probe context", category: "project", - tool_count: 3, + tool_count: 4, }, ToolsetMeta { name: "sch_components", diff --git a/crates/konnect-core/src/tools/editor_navigation.rs b/crates/konnect-core/src/tools/editor_navigation.rs index 3ee88805..e7260b33 100644 --- a/crates/konnect-core/src/tools/editor_navigation.rs +++ b/crates/konnect-core/src/tools/editor_navigation.rs @@ -9,8 +9,8 @@ use crate::mcp::{error::ToolErrorKind, protocol::CallToolResult}; use crate::tool; use crate::tools::{invalid_arg, opt_str, require_array, require_str, ToolContext, ToolDef}; use konnect_ipc::{ - IpcEditorDocument, IpcEditorKind, IpcProjectIdentity, IpcSelectionObservationErrorKind, - IpcSheetInstancePath, + IpcEditorDocument, IpcEditorKind, IpcProjectIdentity, IpcSelectionMutation, + IpcSelectionMutationErrorKind, IpcSelectionObservationErrorKind, IpcSheetInstancePath, }; use serde_json::json; use std::path::PathBuf; @@ -71,6 +71,25 @@ pub fn tools() -> Vec { }), |args, ctx| async move { handle_resolve_navigation_target(args, ctx).await } ), + tool!( + "mutate_editor_selection", + "Clear, add to, or remove from one exact KiCad editor selection. Every non-clear KIID is first resolved in the explicit saved project/document/sheet, and success is derived only from a fresh exact GetSelection readback.", + json!({ + "type": "object", + "properties": { + "operation": { "type": "string", "enum": ["clear", "add", "remove"] }, + "editor": { "type": "string", "enum": ["schematic", "pcb"] }, + "project_name": { "type": "string" }, + "project_path": { "type": "string" }, + "document_path": { "type": "string", "description": "Exact saved .kicad_sch or .kicad_pcb document" }, + "sheet_instance_path": { "type": "array", "items": { "type": "string" } }, + "sheet_path_human_readable": { "type": "string" }, + "object_kiids": { "type": "array", "items": { "type": "string" }, "description": "Empty for clear; one or more stable KIIIDs for add/remove" } + }, + "required": ["operation", "editor", "project_name", "project_path", "document_path", "object_kiids"] + }), + |args, ctx| async move { handle_mutate_editor_selection(args, ctx).await } + ), ] } @@ -452,6 +471,222 @@ fn navigation_target_error_result( CallToolResult::error_kind(kind, error.to_string()) } +#[derive(Debug)] +struct SelectionMutationRequest { + operation: IpcSelectionMutation, + live_document: IpcEditorDocument, + object_kiids: Vec, + structural_targets: Vec, +} + +async fn handle_mutate_editor_selection( + args: &serde_json::Value, + ctx: &ToolContext, +) -> anyhow::Result { + let request = match parse_selection_mutation_request(args) { + Ok(request) => request, + Err(result) => return Ok(result), + }; + let address = ctx.config.ipc_address.clone(); + if address.is_empty() { + return Ok(editor_unavailable("no KiCad IPC endpoint is configured")); + } + let result = tokio::task::spawn_blocking(move || -> anyhow::Result<_> { + let resolved_targets = request + .structural_targets + .iter() + .map(resolve_navigation_target) + .collect::, _>>()?; + let mutation = konnect_ipc::KiCadIpcClient::new(address).mutate_selection( + &request.live_document, + request.operation, + &request.object_kiids, + )?; + Ok((resolved_targets, mutation)) + }) + .await?; + match result { + Ok((resolved_targets, mutation)) => Ok(CallToolResult::json(&json!({ + "resolved_targets": resolved_targets, + "mutation": mutation + }))), + Err(error) => Ok(selection_mutation_error_result(error)), + } +} + +fn parse_selection_mutation_request( + args: &serde_json::Value, +) -> Result { + let operation = match require_str(args, "operation")? { + "clear" => IpcSelectionMutation::Clear, + "add" => IpcSelectionMutation::Add, + "remove" => IpcSelectionMutation::Remove, + _ => { + return Err(invalid_arg( + "operation", + "expected 'clear', 'add', or 'remove'", + )) + } + }; + let editor = match require_str(args, "editor")? { + "schematic" => IpcEditorKind::Schematic, + "pcb" => IpcEditorKind::Pcb, + _ => return Err(invalid_arg("editor", "expected 'schematic' or 'pcb'")), + }; + let project_name = require_str(args, "project_name")?; + let project_path = require_str(args, "project_path")?; + let document_path = require_str(args, "document_path")?; + if project_name.is_empty() || project_path.is_empty() || document_path.is_empty() { + return Err(invalid_arg( + "project_name", + "project and document identity strings must not be empty", + )); + } + let object_kiids = require_array(args, "object_kiids")? + .iter() + .map(|value| value.as_str().map(str::to_string)) + .collect::>>() + .ok_or_else(|| invalid_arg("object_kiids", "every entry must be a string"))?; + if object_kiids.iter().any(String::is_empty) { + return Err(invalid_arg("object_kiids", "KIIIDs must not be empty")); + } + let unique = object_kiids + .iter() + .collect::>(); + if unique.len() != object_kiids.len() { + return Err(invalid_arg( + "object_kiids", + "duplicate KIIIDs are not allowed", + )); + } + match operation { + IpcSelectionMutation::Clear if !object_kiids.is_empty() => { + return Err(invalid_arg("object_kiids", "clear requires an empty array")); + } + IpcSelectionMutation::Add | IpcSelectionMutation::Remove if object_kiids.is_empty() => { + return Err(invalid_arg( + "object_kiids", + "add and remove require at least one KIID", + )); + } + _ => {} + } + + let project = IpcProjectIdentity { + name: project_name.to_string(), + path: project_path.to_string(), + }; + let sheet_instance_path = match editor { + IpcEditorKind::Pcb => { + if !args["sheet_instance_path"].is_null() + || !args["sheet_path_human_readable"].is_null() + { + return Err(invalid_arg( + "sheet_instance_path", + "PCB targets cannot carry schematic sheet identity", + )); + } + None + } + IpcEditorKind::Schematic => { + let ids = require_array(args, "sheet_instance_path")? + .iter() + .map(|value| value.as_str().map(str::to_string)) + .collect::>>() + .ok_or_else(|| { + invalid_arg("sheet_instance_path", "every entry must be a string") + })?; + if ids.is_empty() || ids.iter().any(String::is_empty) { + return Err(invalid_arg( + "sheet_instance_path", + "must contain non-empty root-to-leaf KIIIDs", + )); + } + Some(IpcSheetInstancePath { + kiids: ids, + human_readable: opt_str(args, "sheet_path_human_readable") + .unwrap_or("") + .to_string(), + }) + } + }; + let saved_document = PathBuf::from(document_path); + let structural_targets = object_kiids + .iter() + .map(|kiid| NavigationTargetRequest { + editor, + project: project.clone(), + document_path: saved_document.clone(), + sheet_instance_path: sheet_instance_path.clone(), + object_kiid: Some(kiid.clone()), + human_reference: None, + }) + .collect(); + Ok(SelectionMutationRequest { + operation, + live_document: IpcEditorDocument { + editor, + project: Some(project), + document_path: (editor == IpcEditorKind::Pcb) + .then(|| saved_document.display().to_string()), + sheet_instance_path, + }, + object_kiids, + structural_targets, + }) +} + +fn selection_mutation_error_result(error: anyhow::Error) -> CallToolResult { + if let Some(target) = error + .chain() + .find_map(|cause| cause.downcast_ref::()) + { + return navigation_target_error_result(target.clone()); + } + if let Some(mutation) = konnect_ipc::IpcSelectionMutationError::from_error(&error) { + let kind = match mutation.kind { + IpcSelectionMutationErrorKind::InvalidRequest => ToolErrorKind::InvalidArgument { + field: "object_kiids".to_string(), + reason: mutation.reason.clone(), + }, + IpcSelectionMutationErrorKind::ReadbackMismatch => ToolErrorKind::ReadbackMismatch { + operation: mutation.operation.as_str().to_string(), + requested_kiids: mutation.requested_kiids.clone(), + before_kiids: mutation.before_kiids.clone(), + after_kiids: mutation.after_kiids.clone(), + }, + }; + return CallToolResult::error_kind(kind, mutation.to_string()); + } + if konnect_ipc::IpcSelectionObservationError::from_error(&error).is_some() { + return selection_error_result(error); + } + if let Some(status) = konnect_ipc::ApiStatusError::from_error(&error) { + if status.is_unsupported() { + return CallToolResult::error_kind( + ToolErrorKind::UnsupportedCapability { + capability: "selection_mutation".to_string(), + kicad_version: None, + }, + "The running KiCad endpoint does not support typed selection mutation.", + ); + } + } + match konnect_ipc::IpcFailure::from_error(error) { + konnect_ipc::IpcFailure::Unreachable(_) => { + editor_unavailable("the configured KiCad IPC endpoint is unreachable") + } + _ => CallToolResult::error_kind( + ToolErrorKind::StaleTarget { + target: "requested editor selection mutation".to_string(), + reason: "KiCad did not return a complete typed mutation/readback sequence" + .to_string(), + }, + "KiCad did not return a complete typed selection mutation/readback sequence.", + ), + } +} + #[cfg(test)] mod tests { use super::*; @@ -652,10 +887,87 @@ mod tests { url } + fn spawn_add_selection_mock(project_path: String, apply_mutation: bool) -> String { + static NEXT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + let url = format!( + "inproc://selection-mutation-core-{}", + NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + ); + let socket = nng::Socket::new(nng::Protocol::Rep0).expect("mock socket"); + socket.listen(&url).expect("listen"); + std::thread::spawn(move || { + let mut selected = false; + for _ in 0..8 { + let message = socket.recv().expect("request"); + let request = + kiapi::common::ApiRequest::decode(message.as_slice()).expect("decode request"); + let command = request.message.expect("command"); + let response_any = if command.type_url.ends_with("GetOpenDocuments") { + builders::pack_any( + &kiapi::common::commands::GetOpenDocumentsResponse { + documents: vec![kiapi::common::types::DocumentSpecifier { + r#type: kiapi::common::types::DocumentType::DoctypePcb as i32, + identifier: Some( + kiapi::common::types::document_specifier::Identifier::BoardFilename( + "layout.kicad_pcb".to_string(), + ), + ), + project: Some(kiapi::common::types::ProjectSpecifier { + name: "nav".to_string(), + path: project_path.clone(), + }), + }], + }, + "kiapi.common.commands.GetOpenDocumentsResponse", + ) + } else { + if command.type_url.ends_with("AddToSelection") && apply_mutation { + let add = kiapi::common::commands::AddToSelection::decode( + command.value.as_slice(), + ) + .expect("add selection"); + assert_eq!(add.items[0].value, "fp-c10"); + selected = true; + } + let items = selected + .then(|| { + builders::pack_any( + &kiapi::board::types::FootprintInstance { + id: Some(kiapi::common::types::Kiid { + value: "fp-c10".to_string(), + }), + ..Default::default() + }, + "kiapi.board.types.FootprintInstance", + ) + }) + .into_iter() + .collect(); + builders::pack_any( + &kiapi::common::commands::SelectionResponse { items }, + "kiapi.common.commands.SelectionResponse", + ) + }; + let response = kiapi::common::ApiResponse { + status: Some(kiapi::common::ApiResponseStatus { + status: kiapi::common::ApiStatusCode::AsOk as i32, + error_message: String::new(), + }), + header: None, + message: Some(response_any), + }; + socket + .send(nng::Message::from(response.encode_to_vec().as_slice())) + .expect("response"); + } + }); + url + } + #[test] fn public_tool_is_read_only_and_takes_no_required_arguments() { let definitions = tools(); - assert_eq!(definitions.len(), 3); + assert_eq!(definitions.len(), 4); let state = definitions .iter() .find(|tool| tool.name == "get_editor_state") @@ -677,6 +989,21 @@ mod tests { resolver.input_schema["required"], json!(["editor", "project_name", "project_path", "document_path"]) ); + let mutation = definitions + .iter() + .find(|tool| tool.name == "mutate_editor_selection") + .expect("selection mutation tool"); + assert_eq!( + mutation.input_schema["required"], + json!([ + "operation", + "editor", + "project_name", + "project_path", + "document_path", + "object_kiids" + ]) + ); } #[tokio::test] @@ -845,4 +1172,116 @@ mod tests { "kicad_ipc_get_open_documents" ); } + + #[test] + fn mutation_parser_requires_operation_appropriate_unique_kiids() { + let base = json!({ + "operation": "clear", + "editor": "pcb", + "project_name": "navigation", + "project_path": r"C:\design", + "document_path": r"C:\design\navigation.kicad_pcb", + "object_kiids": [] + }); + assert!(parse_selection_mutation_request(&base).is_ok()); + + for object_kiids in [json!(["a"]), json!(["a", "a"])] { + let mut invalid = base.clone(); + invalid["object_kiids"] = object_kiids; + if invalid["object_kiids"].as_array().map(Vec::len) == Some(2) { + invalid["operation"] = json!("add"); + } + let result = parse_selection_mutation_request(&invalid).expect_err("invalid KIIIDs"); + assert_eq!( + extract_error_kind(&result).as_deref(), + Some("invalid_argument") + ); + } + } + + #[tokio::test] + async fn public_selection_mutation_success_is_derived_from_readback() { + let temp = tempfile::tempdir().unwrap(); + std::fs::write(temp.path().join("nav.kicad_pro"), "{}").unwrap(); + let board = temp.path().join("layout.kicad_pcb"); + std::fs::write( + &board, + "(kicad_pcb (footprint \"Capacitor:C\" (layer \"F.Cu\") (at 1 2) \ + (uuid \"fp-c10\") (property \"Reference\" \"C10\")))", + ) + .unwrap(); + let project_path = temp.path().display().to_string(); + let result = handle_mutate_editor_selection( + &json!({ + "operation": "add", + "editor": "pcb", + "project_name": "nav", + "project_path": project_path.clone(), + "document_path": board.display().to_string(), + "object_kiids": ["fp-c10"] + }), + &context(spawn_add_selection_mock(project_path, true)), + ) + .await + .expect("handler result"); + assert!(!result.is_error); + let ToolContent::Text { text } = &result.content[0] else { + panic!("expected text result"); + }; + let body: serde_json::Value = serde_json::from_str(text).unwrap(); + assert_eq!(body["resolved_targets"][0]["object"]["kiid"], "fp-c10"); + assert_eq!( + body["mutation"]["after"]["selected_objects"][0]["kiid"], + "fp-c10" + ); + assert_eq!( + body["mutation"]["evidence_source"], + "kicad_ipc_selection_mutation_with_get_selection_readback" + ); + } + + #[tokio::test] + async fn public_selection_mutation_reports_readback_mismatch() { + let temp = tempfile::tempdir().unwrap(); + std::fs::write(temp.path().join("nav.kicad_pro"), "{}").unwrap(); + let board = temp.path().join("layout.kicad_pcb"); + std::fs::write( + &board, + "(kicad_pcb (footprint \"Capacitor:C\" (layer \"F.Cu\") (at 1 2) \ + (uuid \"fp-c10\") (property \"Reference\" \"C10\")))", + ) + .unwrap(); + let project_path = temp.path().display().to_string(); + let result = handle_mutate_editor_selection( + &json!({ + "operation": "add", + "editor": "pcb", + "project_name": "nav", + "project_path": project_path.clone(), + "document_path": board.display().to_string(), + "object_kiids": ["fp-c10"] + }), + &context(spawn_add_selection_mock(project_path, false)), + ) + .await + .expect("handler result"); + assert_eq!( + extract_error_kind(&result).as_deref(), + Some("readback_mismatch") + ); + } + + #[test] + fn unsupported_selection_mutation_maps_to_a_typed_capability_refusal() { + let result = + selection_mutation_error_result(anyhow::Error::new(konnect_ipc::ApiStatusError { + code: kiapi::common::ApiStatusCode::AsUnhandled as i32, + code_name: "AS_UNHANDLED".to_string(), + message: "unsupported".to_string(), + })); + assert_eq!( + extract_error_kind(&result).as_deref(), + Some("unsupported_capability") + ); + } } diff --git a/crates/konnect-ipc/src/client.rs b/crates/konnect-ipc/src/client.rs index 534f7edb..2b5f4a6c 100644 --- a/crates/konnect-ipc/src/client.rs +++ b/crates/konnect-ipc/src/client.rs @@ -13,6 +13,7 @@ use crate::types::*; use anyhow::{Context, Result}; // NNG SetOpt trait is brought in scope automatically by the nng crate's prelude use prost::Message; +use std::collections::BTreeSet; use std::path::{Path, PathBuf}; use tracing::{debug, warn}; @@ -829,6 +830,128 @@ impl KiCadIpcClient { }) } + /// Mutate one exact editor selection and prove the complete resulting set + /// through a fresh typed `GetSelection` observation. + pub fn mutate_selection( + &self, + requested: &IpcEditorDocument, + operation: IpcSelectionMutation, + requested_kiids: &[String], + ) -> Result { + let mut unique = BTreeSet::new(); + if requested_kiids.iter().any(|kiid| kiid.is_empty()) { + return Err(selection_mutation_error( + operation, + requested_kiids, + Vec::new(), + Vec::new(), + IpcSelectionMutationErrorKind::InvalidRequest, + "selection KIIDs must not be empty", + )); + } + if requested_kiids.iter().any(|kiid| !unique.insert(kiid)) { + return Err(selection_mutation_error( + operation, + requested_kiids, + Vec::new(), + Vec::new(), + IpcSelectionMutationErrorKind::InvalidRequest, + "selection mutation contains a duplicate KIID", + )); + } + match operation { + IpcSelectionMutation::Clear if !requested_kiids.is_empty() => { + return Err(selection_mutation_error( + operation, + requested_kiids, + Vec::new(), + Vec::new(), + IpcSelectionMutationErrorKind::InvalidRequest, + "clear selection does not accept object KIIDs", + )); + } + IpcSelectionMutation::Add | IpcSelectionMutation::Remove + if requested_kiids.is_empty() => + { + return Err(selection_mutation_error( + operation, + requested_kiids, + Vec::new(), + Vec::new(), + IpcSelectionMutationErrorKind::InvalidRequest, + "add and remove selection require at least one object KIID", + )); + } + _ => {} + } + + let before = self.observe_selection(requested)?; + let document = self.resolve_selection_document(requested)?; + let items = requested_kiids + .iter() + .map(|kiid| kiapi::common::types::Kiid { + value: kiid.clone(), + }) + .collect::>(); + let response = match operation { + IpcSelectionMutation::Clear => self.send_command( + &kiapi::common::commands::ClearSelection { + header: Some(header_for(document)), + }, + "kiapi.common.commands.ClearSelection", + )?, + IpcSelectionMutation::Add => self.send_command( + &kiapi::common::commands::AddToSelection { + header: Some(header_for(document)), + items, + }, + "kiapi.common.commands.AddToSelection", + )?, + IpcSelectionMutation::Remove => self.send_command( + &kiapi::common::commands::RemoveFromSelection { + header: Some(header_for(document)), + items, + }, + "kiapi.common.commands.RemoveFromSelection", + )?, + }; + let _: kiapi::common::commands::SelectionResponse = + unpack_required(response, "selection mutation")?; + let after = self.observe_selection(requested)?; + + let before_kiids = selection_kiids(&before); + let after_kiids = selection_kiids(&after); + let mut expected = before_kiids.iter().cloned().collect::>(); + match operation { + IpcSelectionMutation::Clear => expected.clear(), + IpcSelectionMutation::Add => expected.extend(requested_kiids.iter().cloned()), + IpcSelectionMutation::Remove => { + for kiid in requested_kiids { + expected.remove(kiid); + } + } + } + let expected_kiids = expected.into_iter().collect::>(); + if expected_kiids != after_kiids { + return Err(selection_mutation_error( + operation, + requested_kiids, + before_kiids, + after_kiids, + IpcSelectionMutationErrorKind::ReadbackMismatch, + "post-operation GetSelection did not match the requested exact set transition", + )); + } + + Ok(IpcSelectionMutationResult { + operation, + requested_kiids: requested_kiids.to_vec(), + before, + after, + evidence_source: "kicad_ipc_selection_mutation_with_get_selection_readback".to_string(), + }) + } + /// Prove that one exact editor/document/sheet identity is currently open. /// /// This is the read-only context gate used by semantic target resolution; @@ -3413,6 +3536,34 @@ fn malformed_selected_object( }) } +fn selection_kiids(observation: &IpcSelectionObservation) -> Vec { + let mut kiids = observation + .selected_objects + .iter() + .map(|object| object.kiid.clone()) + .collect::>(); + kiids.sort(); + kiids +} + +fn selection_mutation_error( + operation: IpcSelectionMutation, + requested_kiids: &[String], + before_kiids: Vec, + after_kiids: Vec, + kind: IpcSelectionMutationErrorKind, + reason: &str, +) -> anyhow::Error { + anyhow::Error::new(IpcSelectionMutationError { + kind, + operation, + requested_kiids: requested_kiids.to_vec(), + before_kiids, + after_kiids, + reason: reason.to_string(), + }) +} + fn editor_capabilities( editor: IpcEditorKind, version: &IpcKiCadVersion, diff --git a/crates/konnect-ipc/src/types.rs b/crates/konnect-ipc/src/types.rs index 23884413..e436bf5e 100644 --- a/crates/konnect-ipc/src/types.rs +++ b/crates/konnect-ipc/src/types.rs @@ -132,6 +132,73 @@ pub struct IpcSelectionObservation { pub evidence_source: String, } +/// Semantic selection change requested from one exact editor context. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum IpcSelectionMutation { + Clear, + Add, + Remove, +} + +impl IpcSelectionMutation { + pub fn as_str(self) -> &'static str { + match self { + Self::Clear => "clear", + Self::Add => "add", + Self::Remove => "remove", + } + } +} + +/// Verified selection mutation. Success means the post-operation observation +/// exactly matched the requested set transition, not merely that IPC replied. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct IpcSelectionMutationResult { + pub operation: IpcSelectionMutation, + pub requested_kiids: Vec, + pub before: IpcSelectionObservation, + pub after: IpcSelectionObservation, + pub evidence_source: String, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum IpcSelectionMutationErrorKind { + InvalidRequest, + ReadbackMismatch, +} + +/// A semantic selection mutation was invalid or its observed result did not +/// prove the requested change. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct IpcSelectionMutationError { + pub kind: IpcSelectionMutationErrorKind, + pub operation: IpcSelectionMutation, + pub requested_kiids: Vec, + pub before_kiids: Vec, + pub after_kiids: Vec, + pub reason: String, +} + +impl IpcSelectionMutationError { + pub fn from_error(error: &anyhow::Error) -> Option<&Self> { + error.chain().find_map(|cause| cause.downcast_ref::()) + } +} + +impl std::fmt::Display for IpcSelectionMutationError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + formatter, + "cannot verify {} selection mutation: {}", + self.operation.as_str(), + self.reason + ) + } +} + +impl std::error::Error for IpcSelectionMutationError {} + /// Stable classification for a fail-closed selection observation. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum IpcSelectionObservationErrorKind { diff --git a/crates/konnect-ipc/tests/mock_server_test.rs b/crates/konnect-ipc/tests/mock_server_test.rs index 75018714..7b8de461 100644 --- a/crates/konnect-ipc/tests/mock_server_test.rs +++ b/crates/konnect-ipc/tests/mock_server_test.rs @@ -8,7 +8,8 @@ use konnect_ipc::builders; use konnect_ipc::gen::kiapi; use konnect_ipc::{ - IpcEditorDocument, IpcEditorKind, IpcProjectIdentity, IpcSelectionObservationError, + IpcEditorDocument, IpcEditorKind, IpcProjectIdentity, IpcSelectionMutation, + IpcSelectionMutationError, IpcSelectionMutationErrorKind, IpcSelectionObservationError, IpcSelectionObservationErrorKind, IpcSheetInstancePath, KiCadIpcClient, }; use nng::options::Options; @@ -2535,3 +2536,156 @@ fn document_disappearing_during_selection_readback_is_stale_editor_state() { IpcSelectionObservationErrorKind::StaleEditorState ); } + +fn selected_footprints(ids: &std::collections::BTreeSet) -> Vec { + ids.iter() + .map(|id| { + builders::pack_any( + &kiapi::board::types::FootprintInstance { + id: Some(kiid(id)), + ..Default::default() + }, + "kiapi.board.types.FootprintInstance", + ) + }) + .collect() +} + +fn spawn_selection_mutation_mock(initial: &[&str], apply_mutation: bool) -> MockKicad { + let selection = Arc::new(Mutex::new( + initial + .iter() + .map(|id| (*id).to_string()) + .collect::>(), + )); + let selection_in_mock = selection.clone(); + spawn_mock(move |request| { + let message = request.message.expect("command"); + if message.type_url.ends_with("GetOpenDocuments") { + return Some(open_navigation_documents_response(vec![ + navigation_board_document(), + ])); + } + if message.type_url.ends_with("GetSelection") { + return Some(selection_response(selected_footprints( + &selection_in_mock.lock().unwrap(), + ))); + } + + let mutation_header = if message.type_url.ends_with("ClearSelection") { + let command = + kiapi::common::commands::ClearSelection::decode(message.value.as_slice()).unwrap(); + if apply_mutation { + selection_in_mock.lock().unwrap().clear(); + } + command.header + } else if message.type_url.ends_with("AddToSelection") { + let command = + kiapi::common::commands::AddToSelection::decode(message.value.as_slice()).unwrap(); + if apply_mutation { + selection_in_mock + .lock() + .unwrap() + .extend(command.items.iter().map(|id| id.value.clone())); + } + command.header + } else if message.type_url.ends_with("RemoveFromSelection") { + let command = + kiapi::common::commands::RemoveFromSelection::decode(message.value.as_slice()) + .unwrap(); + if apply_mutation { + let mut selection = selection_in_mock.lock().unwrap(); + for id in &command.items { + selection.remove(&id.value); + } + } + command.header + } else { + panic!("unexpected request {}", message.type_url); + }; + let document = mutation_header + .as_ref() + .and_then(|header| header.document.as_ref()) + .expect("selection mutation document"); + assert_eq!(board_filename(document), "navigation.kicad_pcb"); + Some(selection_response(selected_footprints( + &selection_in_mock.lock().unwrap(), + ))) + }) +} + +#[test] +fn clear_add_and_remove_selection_are_proven_by_exact_readback() { + let cases = [ + (IpcSelectionMutation::Clear, vec!["a"], Vec::::new()), + ( + IpcSelectionMutation::Add, + vec!["a"], + vec!["a".to_string(), "b".to_string()], + ), + ( + IpcSelectionMutation::Remove, + vec!["a", "b"], + vec!["a".to_string()], + ), + ]; + for (operation, initial, expected) in cases { + let mock = spawn_selection_mutation_mock(&initial, true); + let requested = match operation { + IpcSelectionMutation::Clear => Vec::new(), + IpcSelectionMutation::Add | IpcSelectionMutation::Remove => vec!["b".to_string()], + }; + let result = KiCadIpcClient::new(&mock.url) + .mutate_selection(&navigation_board_target(), operation, &requested) + .expect("verified selection mutation"); + let mut observed = result + .after + .selected_objects + .iter() + .map(|object| object.kiid.clone()) + .collect::>(); + observed.sort(); + assert_eq!(observed, expected); + assert_eq!(result.operation, operation); + assert!(result.evidence_source.contains("get_selection_readback")); + } +} + +#[test] +fn transport_success_without_the_requested_selection_change_is_a_mismatch() { + let mock = spawn_selection_mutation_mock(&["a"], false); + let error = KiCadIpcClient::new(&mock.url) + .mutate_selection( + &navigation_board_target(), + IpcSelectionMutation::Add, + &["b".to_string()], + ) + .expect_err("unchanged readback is not success"); + let typed = IpcSelectionMutationError::from_error(&error).expect("typed mutation error"); + assert_eq!(typed.kind, IpcSelectionMutationErrorKind::ReadbackMismatch); + assert_eq!(typed.before_kiids, ["a"]); + assert_eq!(typed.after_kiids, ["a"]); +} + +#[test] +fn invalid_selection_mutations_are_rejected_before_transport() { + let client = KiCadIpcClient::new("inproc://not-contacted"); + for (operation, requested) in [ + (IpcSelectionMutation::Clear, vec!["a".to_string()]), + (IpcSelectionMutation::Add, Vec::new()), + ( + IpcSelectionMutation::Remove, + vec!["a".to_string(), "a".to_string()], + ), + ] { + let error = client + .mutate_selection(&navigation_board_target(), operation, &requested) + .expect_err("invalid request"); + assert_eq!( + IpcSelectionMutationError::from_error(&error) + .expect("typed mutation error") + .kind, + IpcSelectionMutationErrorKind::InvalidRequest + ); + } +} diff --git a/docs/KICAD_INTEGRATION.md b/docs/KICAD_INTEGRATION.md index 989bb150..e34d1d6b 100644 --- a/docs/KICAD_INTEGRATION.md +++ b/docs/KICAD_INTEGRATION.md @@ -71,6 +71,14 @@ object matches in the exact document and sheet instance; duplicates return structured candidates, and stale project ownership or symbol-instance paths fail closed before any editor mutation. +Selection mutation uses KiCad's typed `ClearSelection`, `AddToSelection`, and +`RemoveFromSelection` commands only after every non-clear KIID resolves in the +explicit saved project/document/sheet. The transport response is not treated +as success: Konnect performs a fresh exact-context `GetSelection`, compares the +entire observed set with the expected before/after transition, and returns a +structured `readback_mismatch` if KiCad did not make precisely that change. +Duplicate or empty KIID requests are rejected before IPC. + ## Schematic-To-Board Sync `update_pcb_from_schematic` in `tools/pcb_sync.rs` is live-IPC-only. It uses diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 4dd58ae4..06c050ce 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -309,7 +309,7 @@ callable tools, the fix is to make the *first* listing complete: ``` in `konnect.toml` in the working directory, or a `settings.json` beside the binary. Every toolset is then loaded at -startup, so `tools/list` carries all 231 tools from the first call. +startup, so `tools/list` carries all 232 tools from the first call. It is off by default because it costs what the router exists to save: roughly 25K tokens per listing instead of ~2K. Turn it on only if your client needs it. diff --git a/packaging/metadata.json b/packaging/metadata.json index ccb6bdd3..064cbd2b 100644 --- a/packaging/metadata.json +++ b/packaging/metadata.json @@ -1,7 +1,7 @@ { "$schema": "https://go.kicad.org/pcm/schemas/v1", "name": "Konnect", - "description": "AI-assisted PCB design via the Model Context Protocol. Enables Claude and other AI assistants to design schematics and PCBs with 224 tools organized into on-demand toolsets.", + "description": "AI-assisted PCB design via the Model Context Protocol. Enables Claude and other AI assistants to design schematics and PCBs with 225 tools organized into on-demand toolsets.", "description_full": "Konnect exposes a complete set of KiCAD design tools to AI assistants via the Model Context Protocol (MCP). It supports schematic editing, PCB layout, local Freerouting MCP routing, library management, JLCPCB part search, ERC/DRC, design review audits, and full export pipelines. Tools are organized into 21 toolsets loaded on demand so the AI only sees relevant tools at once.", "identifier": "com.github.mixelpixx.konnect", "type": "plugin", diff --git a/plugin/plugin.json b/plugin/plugin.json index db7c9142..2ccedd63 100644 --- a/plugin/plugin.json +++ b/plugin/plugin.json @@ -1,7 +1,7 @@ { "identifier": "com.github.mixelpixx.konnect", "name": "Konnect", - "description": "AI-assisted PCB design via the Model Context Protocol. 224 tools for schematic editing, PCB layout, routing, design review, and manufacturing export.", + "description": "AI-assisted PCB design via the Model Context Protocol. 225 tools for schematic editing, PCB layout, routing, design review, and manufacturing export.", "runtime": { "type": "exec" }, diff --git a/tool-directory.md b/tool-directory.md index 4118a6d3..7dcb4057 100644 --- a/tool-directory.md +++ b/tool-directory.md @@ -13,7 +13,7 @@ Compatibility notes for removed or narrowed arguments are recorded in ## Overview - **21 toolsets** organized into 10 categories -- **224 registered tools** + **7 always-visible meta-tools** = **231 total** +- **225 registered tools** + **7 always-visible meta-tools** = **232 total** - **Discovery pattern**: the server pre-loads only the **starter kit** (`project`, `config`) so baseline `tools/list` costs ~2K tokens instead of ~23K. The LLM reads `list_toolboxes` → calls `load_toolset(name)` to expose additional tools on demand; `unload_toolset(name)` prunes them. `tools/list_changed` is notified on every mutation. If the LLM calls a tool whose toolset isn't loaded, the error names the owning toolset so recovery is a single `load_toolset` hop. `load_toolset` also accepts an array of names to load several toolsets with a single `tools/list` refresh. - **Observability**: every `tools/call` is recorded — ring buffer of the last 100 calls + per-tool counters + JSONL at `/logs/calls.jsonl`. The LLM self-diagnoses via `get_recent_calls` and `server_stats`. @@ -61,7 +61,7 @@ Seven tools, grouped into *discovery/routing*, *observability*, and *runtime dia | `snapshot_project` | Export the schematic and PCB to PDF as a timestamped snapshot/checkpoint. Useful before major edits. | | `open_schematic_viewer` | Launch the live schematic viewer (SVG with auto-refresh on file change). Use after placing components so the user can see changes in real time. | -### `editor_navigation` · 3 tools +### `editor_navigation` · 4 tools **Purpose:** Observe and semantically navigate exact KiCad editor, document, sheet, selection, and cross-probe context. **Source:** [`crates/konnect-core/src/tools/editor_navigation.rs`](crates/konnect-core/src/tools/editor_navigation.rs) @@ -70,6 +70,7 @@ Seven tools, grouped into *discovery/routing*, *observability*, and *runtime dia | `get_editor_state` | Observe the configured KiCad IPC endpoint's running version, addressable schematic/PCB editors, exact open document identities, capability availability, and explicit active-context limitations. | | `get_editor_selection` | Read the selection from one exact editor, project, document, and hierarchical sheet instance with stable KIID/UUID identities and document-readback freshness checks. | | `resolve_navigation_target` | Resolve an exact open project/document/sheet object by stable KIID, or by a human reference only when saved KiCad structure yields one unambiguous candidate. | +| `mutate_editor_selection` | Clear, add to, or remove from one exact editor selection after saved-object validation; report success only when a fresh typed selection readback proves the complete requested set transition. | ---