diff --git a/DEV.md b/DEV.md index 88905a39..1d5c100b 100644 --- a/DEV.md +++ b/DEV.md @@ -303,7 +303,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 216 tools (222 total with the 6 meta-tools) in `tools/list` by default — that would cost ~23K tokens of context on every listing. Instead: +The server does NOT expose all 217 tools (223 total with the 6 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 6 meta-tools, baseline `tools/list` is 20 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. @@ -378,9 +378,9 @@ convention for other `kicad-cli`-calling code. ## Current Stats -- **20 toolsets, 216 tools** + 6 meta-tools (4 routing + 2 observability — see `tool-directory.md`) +- **20 toolsets, 217 tools** + 6 meta-tools (4 routing + 2 observability — see `tool-directory.md`) - Baseline `tools/list`: 20 tools / ~2K tokens (starter kit + meta-tools) -- Full-catalog `tools/list` (all loaded): 222 tools (216 registered + 6 meta) / ~25K tokens +- Full-catalog `tools/list` (all loaded): 223 tools (217 registered + 6 meta) / ~25K tokens - **0 IPC stubs** (all protobuf methods implemented) - **0 unimplemented tools** - **Specctra DSN/SES are PCB-editor operations**, not `kicad-cli` commands. Konnect diff --git a/README.md b/README.md index faaa1bed..14b146cf 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). -**216 tools across 20 on-demand toolsets.** Schematic capture, PCB layout and +**217 tools across 20 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 216 tools to an LLM costs roughly 23K +**Context economy is a feature.** Exposing all 217 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/router/registry.rs b/crates/konnect-core/src/router/registry.rs index 28c6a803..f78e5104 100644 --- a/crates/konnect-core/src/router/registry.rs +++ b/crates/konnect-core/src/router/registry.rs @@ -70,7 +70,7 @@ pub static ALL_TOOLSETS: &[ToolsetMeta] = &[ name: "pcb_board", description: "Board outline, layers, zones, mounting holes, board text, SVG logo import", category: "pcb", - tool_count: 11, + tool_count: 12, }, ToolsetMeta { name: "pcb_components", diff --git a/crates/konnect-core/src/tools/mod.rs b/crates/konnect-core/src/tools/mod.rs index 2a5e0a71..1f284022 100644 --- a/crates/konnect-core/src/tools/mod.rs +++ b/crates/konnect-core/src/tools/mod.rs @@ -333,6 +333,23 @@ pub fn opt_str<'a>(args: &'a Value, key: &str) -> Option<&'a str> { args[key].as_str() } +/// Extract an optional array-of-strings argument: `None` when absent, a +/// structured `InvalidArgument` error when present but not an array of +/// strings. Prefer this over `as_array().unwrap_or_default()`, which reports a +/// malformed argument as an empty list. +pub fn opt_str_list(args: &Value, key: &str) -> Result>, CallToolResult> { + match &args[key] { + Value::Null => Ok(None), + Value::Array(values) => values + .iter() + .map(|value| value.as_str().map(str::to_string)) + .collect::>>() + .map(Some) + .ok_or_else(|| invalid_arg(key, "every entry must be a string")), + _ => Err(invalid_arg(key, "expected an array of strings")), + } +} + /// Extract a required f64 argument. Returns a structured `InvalidArgument` /// error result if missing or not a number. pub fn require_f64(args: &Value, key: &str) -> Result { diff --git a/crates/konnect-core/src/tools/pcb_board.rs b/crates/konnect-core/src/tools/pcb_board.rs index c799035a..c0f9fe4e 100644 --- a/crates/konnect-core/src/tools/pcb_board.rs +++ b/crates/konnect-core/src/tools/pcb_board.rs @@ -6,9 +6,12 @@ //! the file is the last save, so it disagrees with the IPC-backed writers here //! whenever KiCad holds unsaved edits. +use crate::mcp::error::ToolErrorKind; use crate::mcp::protocol::CallToolResult; use crate::tool; -use crate::tools::{get_path, require_f64, require_str, ToolContext, ToolDef}; +use crate::tools::{ + get_path, opt_str_list, require_f64, require_str, with_ipc_classified, ToolContext, ToolDef, +}; use konnect_ipc::builders; use konnect_sexp::{ parser::{parse_sexp, SexpNode}, @@ -375,6 +378,133 @@ pub(crate) fn zone_schema() -> serde_json::Value { }) } +// ─── Board graphics ─────────────────────────────────────────────────────────── + +/// The graphic kinds `delete_graphics` names, in the vocabulary both the live +/// and the file reader answer in. +/// +/// `shape` is the live path's fallback for a shape KiCad sends with no +/// geometry (`konnect_ipc::client::shape_kind_and_origin`); it is in the list +/// so that everything the tool can *report* is also something a `types` filter +/// can *select* — otherwise such an item would be undeletable by kind. +const GRAPHIC_KINDS: [&str; 10] = [ + "line", + "rect", + "arc", + "circle", + "poly", + "curve", + "shape", + "text", + "textbox", + "dimension", +]; + +/// The kind name for a board file's top-level graphic block, or `None` for +/// anything that is not a graphic — footprints, zones, tracks, the setup +/// block. `(image …)` is deliberately absent: KiCad 10's `ReferenceImage` +/// message is an empty placeholder, so the live path cannot see one and +/// deleting it from the file only would make the two paths disagree. +fn graphic_kind(tag: &str) -> Option<&'static str> { + Some(match tag { + "gr_line" => "line", + "gr_rect" => "rect", + "gr_arc" => "arc", + "gr_circle" => "circle", + "gr_poly" => "poly", + "gr_curve" => "curve", + "gr_text" => "text", + "gr_text_box" => "textbox", + "dimension" => "dimension", + _ => return None, + }) +} + +/// The head tag of a `(tag …)` block, without parsing it. +fn block_tag(block: &str) -> Option<&str> { + let after_paren = block.strip_prefix('(')?; + let end = after_paren + .find(|c: char| c.is_whitespace() || c == '(' || c == ')') + .unwrap_or(after_paren.len()); + Some(&after_paren[..end]) +} + +/// A graphic read out of a board file, with the byte range to cut to delete it. +struct FileGraphic { + uuid: String, + kind: &'static str, + layer: String, + origin: Option<(f64, f64)>, + /// Byte range of the block including its leading whitespace, so deleting + /// it leaves no blank line behind. + span: (usize, usize), +} + +/// The first defining point of a graphic block: a segment's `start`, a +/// circle's `center`, a text's `at`, or a polygon's first vertex. +fn block_origin(node: &SexpNode) -> Option<(f64, f64)> { + for tag in ["start", "center", "at"] { + if let Some(point) = node.find(tag) { + if let (Some(x), Some(y)) = (point.get_f64(1), point.get_f64(2)) { + return Some((x, y)); + } + } + } + let xy = node.find("pts")?.find("xy")?; + Some((xy.get_f64(1)?, xy.get_f64(2)?)) +} + +/// Every top-level graphic in a board file, in file order. +/// +/// Only direct children of `(kicad_pcb …)` count: a `gr_line` inside a +/// footprint belongs to that footprint, and this tool must never cut one out. +fn read_file_graphics(content: &str) -> Vec { + find_direct_child_blocks(content, "kicad_pcb") + .into_iter() + .filter_map(|(start, end)| { + // Read the head tag out of the slice before parsing: a board's + // top-level blocks are overwhelmingly footprints, segments, vias, + // and zones, and building an AST for each of those just to throw + // it away parses most of the file for nothing. + let kind = graphic_kind(block_tag(&content[start..end])?)?; + let node = parse_sexp(&content[start..end]).ok()?; + Some(FileGraphic { + uuid: node.find_str("uuid").unwrap_or_default().to_string(), + kind, + layer: node.find_str("layer").unwrap_or_default().to_string(), + origin: block_origin(&node), + span: find_block_with_leading_whitespace(content, start).unwrap_or((start, end)), + }) + }) + .collect() +} + +/// The `delete_graphics` filter: a graphic matches when it satisfies every +/// filter given (unset filters match everything). +#[derive(Clone)] +struct GraphicFilter { + uuids: Option>, + kinds: Option>, + layer: Option, +} + +impl GraphicFilter { + fn matches(&self, uuid: &str, kind: &str, layer: &str) -> bool { + self.uuids + .as_ref() + .is_none_or(|wanted| wanted.iter().any(|w| w == uuid)) + && self + .kinds + .as_ref() + .is_none_or(|wanted| wanted.iter().any(|w| w == kind)) + && self.layer.as_ref().is_none_or(|wanted| wanted == layer) + } + + fn is_empty(&self) -> bool { + self.uuids.is_none() && self.kinds.is_none() && self.layer.is_none() + } +} + // ─── S-expression format helpers ────────────────────────────────────────────── fn format_gr_line(x1: f64, y1: f64, x2: f64, y2: f64, layer: &str, width: f64) -> String { @@ -602,7 +732,10 @@ pub fn tools() -> Vec { vec![ tool!( "set_board_size", - "Set the PCB board outline to a rectangle of the given dimensions on the Edge.Cuts layer.", + "Add a rectangular board outline of the given dimensions on the Edge.Cuts layer. \ + This appends: on a board that already has an outline it leaves two overlapping \ + rectangles and a DRC failure, so resizing means calling \ + delete_graphics(layer='Edge.Cuts') first.", json!({ "type": "object", "properties": { @@ -686,7 +819,10 @@ pub fn tools() -> Vec { ), tool!( "add_board_outline", - "Add a rectangular board outline on Edge.Cuts, optionally using circular rounded corners.", + "Add a rectangular board outline on Edge.Cuts, optionally using circular rounded \ + corners. This appends: on a board that already has an outline it leaves two \ + overlapping rectangles and a DRC failure, so replacing one means calling \ + delete_graphics(layer='Edge.Cuts') first.", json!({ "type": "object", "properties": { @@ -701,6 +837,43 @@ pub fn tools() -> Vec { }), |args, ctx| async move { handle_add_board_outline(args, ctx).await } ), + tool!( + "delete_graphics", + "Delete board graphics — lines, rectangles, arcs, circles, polygons, curves, text, \ + text boxes, and dimensions — that match every filter given. At least one of \ + 'uuids', 'layer', or 'types' is required; 'dry_run' lists what would go without \ + deleting anything, and the UUIDs it reports can be passed straight back as 'uuids'. \ + Footprints, zones, tracks, vias, and graphics belonging to a footprint are never \ + touched, and neither are reference images (KiCad's API cannot identify one). \ + This is how a board outline is resized: add_board_outline and set_board_size \ + append, so calling one twice without deleting the old Edge.Cuts graphics first \ + leaves two overlapping outlines. Acts on the board open in KiCad when it is \ + reachable, else on the file — 'source' says which.", + json!({ + "type": "object", + "properties": { + "board": { "type": "string", "description": "Path to .kicad_pcb file" }, + "layer": { "type": "string", "description": "Only graphics on this layer (e.g. 'Edge.Cuts')" }, + "uuids": { + "type": "array", + "description": "Only graphics with these UUIDs", + "items": { "type": "string" } + }, + "types": { + "type": "array", + "description": "Only graphics of these kinds", + "items": { "type": "string", "enum": GRAPHIC_KINDS } + }, + "dry_run": { + "type": "boolean", + "description": "List the matches without deleting them", + "default": false + } + }, + "required": ["board"] + }), + |args, ctx| async move { handle_delete_graphics(args, ctx).await } + ), tool!( "add_mounting_hole", "Add an NPTH mounting hole footprint at the specified position.", @@ -1413,6 +1586,155 @@ async fn handle_add_board_outline( }))) } +fn graphic_json( + uuid: &str, + kind: &str, + layer: &str, + origin: Option<(f64, f64)>, +) -> serde_json::Value { + json!({ + "uuid": uuid, + "type": kind, + "layer": layer, + "x": origin.map(|(x, _)| x), + "y": origin.map(|(_, y)| y), + }) +} + +async fn handle_delete_graphics( + args: &serde_json::Value, + ctx: &ToolContext, +) -> anyhow::Result { + let board_path = get_path(args, "board")?; + let uuids = match opt_str_list(args, "uuids") { + Ok(v) => v, + Err(e) => return Ok(e), + }; + let kinds = match opt_str_list(args, "types") { + Ok(v) => v, + Err(e) => return Ok(e), + }; + let layer = args["layer"].as_str().map(str::to_string); + let dry_run = args["dry_run"].as_bool().unwrap_or(false); + + if let Some(unknown) = kinds + .iter() + .flatten() + .find(|kind| !GRAPHIC_KINDS.contains(&kind.as_str())) + { + return Ok(CallToolResult::error_kind( + ToolErrorKind::InvalidArgument { + field: "types".to_string(), + reason: format!("unknown graphic type '{unknown}'"), + }, + format!( + "Unknown graphic type '{unknown}'. Valid types: {}.", + GRAPHIC_KINDS.join(", ") + ), + )); + } + + let filter = GraphicFilter { + uuids, + kinds, + layer, + }; + // An unfiltered call would wipe every graphic on the board. That is never + // what a caller means by omitting the arguments, so it has to be spelled + // out — layer by layer, or by UUID. + if filter.is_empty() { + return Ok(CallToolResult::error_kind( + ToolErrorKind::InvalidArgument { + field: "filter".to_string(), + reason: "at least one of 'uuids', 'layer', or 'types' is required".to_string(), + }, + "delete_graphics needs a filter: pass 'layer' (e.g. 'Edge.Cuts'), 'uuids', \ + or 'types'. Run it with dry_run to see what a filter would match." + .to_string(), + )); + } + + // The board KiCad holds first. The fallback gate is the typed transport + // classification: only when the request never reached a live KiCad is it + // safe to edit the board file, since a file edited behind a live editor is + // silently overwritten on its next save. + let ipc_board = board_path.clone(); + let ipc_filter = filter.clone(); + let attempt = with_ipc_classified(ctx.config.ipc_address.clone(), move |c| { + let document = c.find_open_board(&ipc_board)?; + let matched: Vec = c + .get_board_graphics_in(document.clone())? + .into_iter() + .filter(|g| ipc_filter.matches(&g.uuid, &g.kind, &g.layer)) + .collect(); + if !dry_run { + if let Some(anonymous) = matched.iter().find(|g| g.uuid.is_empty()) { + anyhow::bail!( + "KiCad returned a {} on {} with no identifier, so it cannot be deleted; \ + nothing was deleted", + anonymous.kind, + anonymous.layer + ); + } + c.delete_items_in(document, matched.iter().map(|g| g.uuid.clone()).collect())?; + } + Ok(matched) + }) + .await?; + + let (graphics, source) = match attempt { + Ok(matched) => ( + matched + .iter() + .map(|g| { + graphic_json( + &g.uuid, + &g.kind, + &g.layer, + g.origin.as_ref().map(|p| (p.x, p.y)), + ) + }) + .collect::>(), + "ipc", + ), + Err(konnect_ipc::IpcFailure::Rejected(message)) => { + return Ok(CallToolResult::error(format!( + "KiCad rejected the deletion over IPC: {message}. \ + The board file was not modified — KiCad is reachable and may hold this \ + board open, so editing the file directly could be silently overwritten." + ))) + } + Err(konnect_ipc::IpcFailure::Unreachable(_)) => { + let content = std::fs::read_to_string(&board_path)?; + let matched: Vec = read_file_graphics(&content) + .into_iter() + .filter(|g| filter.matches(&g.uuid, g.kind, &g.layer)) + .collect(); + let graphics = matched + .iter() + .map(|g| graphic_json(&g.uuid, g.kind, &g.layer, g.origin)) + .collect::>(); + + if !dry_run && !matched.is_empty() { + let edits = matched + .iter() + .map(|g| SexpEdit::delete(g.span.0, g.span.1)) + .collect(); + write_atomic(&board_path, &apply_edits(content, edits))?; + } + (graphics, "file") + } + }; + + Ok(CallToolResult::json(&json!({ + "count": graphics.len(), + "deleted": if dry_run { 0 } else { graphics.len() }, + "dry_run": dry_run, + "graphics": graphics, + "source": source + }))) +} + async fn handle_add_mounting_hole( args: &serde_json::Value, ctx: &ToolContext, @@ -2044,6 +2366,308 @@ mod svg_logo_tests { } } +/// `add_board_outline` and `set_board_size` only ever appended, so an outline +/// was write-once: a second call left two overlapping rectangles and a DRC +/// failure, and shrinking a board meant deleting the old edges by hand in +/// KiCad. `delete_graphics` is the missing delete verb. +#[cfg(test)] +mod delete_graphics_tests { + use super::board_mock::{ctx_talking_to, spawn_kicad_holding_board}; + use super::*; + use konnect_ipc::gen::kiapi; + use prost::Message; + use std::sync::{Arc, Mutex}; + + /// Tab-indented like KiCad 10's own writer, with a graphic *inside* a + /// footprint that no filter may ever reach. + const BOARD: &str = "(kicad_pcb\n\ + \t(version 20260206)\n\ + \t(paper \"A4\")\n\ + \t(gr_line\n\t\t(start 0 0)\n\t\t(end 100 0)\n\t\t(layer \"Edge.Cuts\")\n\t\t(uuid \"edge-top\")\n\t)\n\ + \t(gr_line\n\t\t(start 100 0)\n\t\t(end 100 60)\n\t\t(layer \"Edge.Cuts\")\n\t\t(uuid \"edge-right\")\n\t)\n\ + \t(gr_text \"REV A\"\n\t\t(at 10 10 0)\n\t\t(layer \"F.SilkS\")\n\t\t(uuid \"silk-text\")\n\t)\n\ + \t(gr_circle\n\t\t(center 5 5)\n\t\t(end 7 5)\n\t\t(layer \"F.SilkS\")\n\t\t(uuid \"silk-dot\")\n\t)\n\ + \t(footprint \"R_0402\"\n\t\t(at 20 20)\n\ + \t\t(gr_line\n\t\t\t(start 0 0)\n\t\t\t(end 1 0)\n\t\t\t(layer \"Edge.Cuts\")\n\t\t\t(uuid \"inside-footprint\")\n\t\t)\n\ + \t\t(uuid \"fp1\")\n\t)\n\ + \t(zone (net 1) (layer \"F.Cu\") (uuid \"zone1\"))\n\ + )\n"; + + async fn delete_graphics( + ctx: &ToolContext, + args: serde_json::Value, + ) -> (serde_json::Value, bool) { + let result = handle_delete_graphics(&args, ctx) + .await + .expect("handler should succeed"); + let body = match &result.content[0] { + crate::mcp::protocol::ToolContent::Text { text } => text.clone(), + other => panic!("expected text content, got {other:?}"), + }; + (serde_json::from_str(&body).unwrap(), result.is_error) + } + + fn offline() -> ToolContext { + ctx_talking_to(String::new()) + } + + fn board_file() -> (tempfile::TempDir, std::path::PathBuf) { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("board.kicad_pcb"); + std::fs::write(&path, BOARD).unwrap(); + (dir, path) + } + + #[tokio::test] + async fn clearing_edge_cuts_leaves_the_rest_of_the_board() { + let (_dir, board) = board_file(); + + let (body, is_error) = delete_graphics( + &offline(), + json!({ + "board": board.to_str().unwrap(), + "layer": "Edge.Cuts" + }), + ) + .await; + + assert!(!is_error, "{body}"); + assert_eq!(body["deleted"], json!(2)); + assert_eq!(body["source"], json!("file")); + + let updated = std::fs::read_to_string(&board).unwrap(); + assert!(!updated.contains("edge-top")); + assert!(!updated.contains("edge-right")); + // A footprint's own graphics are the footprint's, whatever layer they + // claim — cutting one out would corrupt the part. + assert!(updated.contains("inside-footprint")); + assert!(updated.contains("silk-text")); + assert!(updated.contains("zone1")); + parse_sexp(&updated).expect("the board still parses"); + } + + /// The whole point of the tool: place an outline, clear it, place a + /// smaller one, and end up with exactly one rectangle. + #[tokio::test] + async fn an_outline_can_be_replaced_by_clearing_it_first() { + let (_dir, board) = board_file(); + let ctx = offline(); + let args = json!({ "board": board.to_str().unwrap() }); + + delete_graphics( + &ctx, + json!({ "board": board.to_str().unwrap(), "layer": "Edge.Cuts" }), + ) + .await; + let mut outline = args.clone(); + outline["x1"] = json!(0.0); + outline["y1"] = json!(0.0); + outline["x2"] = json!(50.0); + outline["y2"] = json!(30.0); + handle_add_board_outline(&outline, &ctx).await.unwrap(); + + let updated = std::fs::read_to_string(&board).unwrap(); + let edges = read_file_graphics(&updated) + .into_iter() + .filter(|g| g.layer == "Edge.Cuts") + .count(); + assert_eq!(edges, 4, "one rectangle, not two overlapping ones"); + } + + #[tokio::test] + async fn a_dry_run_reports_the_matches_and_changes_nothing() { + let (_dir, board) = board_file(); + + let (body, is_error) = delete_graphics( + &offline(), + json!({ + "board": board.to_str().unwrap(), + "layer": "Edge.Cuts", + "dry_run": true + }), + ) + .await; + + assert!(!is_error, "{body}"); + assert_eq!(body["count"], json!(2)); + assert_eq!(body["deleted"], json!(0)); + assert_eq!(body["graphics"][0]["uuid"], json!("edge-top")); + assert_eq!(body["graphics"][0]["type"], json!("line")); + assert_eq!(body["graphics"][0]["x"], json!(0.0)); + assert_eq!(std::fs::read_to_string(&board).unwrap(), BOARD); + } + + #[tokio::test] + async fn a_uuid_filter_deletes_exactly_that_graphic() { + let (_dir, board) = board_file(); + + let (body, _) = delete_graphics( + &offline(), + json!({ + "board": board.to_str().unwrap(), + "uuids": ["silk-text"] + }), + ) + .await; + + assert_eq!(body["deleted"], json!(1)); + let updated = std::fs::read_to_string(&board).unwrap(); + assert!(!updated.contains("silk-text")); + assert!(updated.contains("silk-dot")); + } + + #[tokio::test] + async fn filters_combine() { + let (_dir, board) = board_file(); + + let (body, _) = delete_graphics( + &offline(), + json!({ + "board": board.to_str().unwrap(), + "layer": "F.SilkS", + "types": ["circle"] + }), + ) + .await; + + assert_eq!(body["deleted"], json!(1)); + assert_eq!(body["graphics"][0]["uuid"], json!("silk-dot")); + assert!(std::fs::read_to_string(&board) + .unwrap() + .contains("silk-text")); + } + + /// Omitting every filter would wipe the board's artwork, which no caller + /// means by leaving the arguments out. + #[tokio::test] + async fn a_call_with_no_filter_is_refused() { + let (_dir, board) = board_file(); + + let (body, is_error) = + delete_graphics(&offline(), json!({ "board": board.to_str().unwrap() })).await; + + assert!(is_error); + assert_eq!(body["error"]["kind"], json!("invalid_argument")); + assert_eq!(std::fs::read_to_string(&board).unwrap(), BOARD); + } + + #[tokio::test] + async fn an_unknown_type_names_the_valid_ones() { + let (_dir, board) = board_file(); + + let (body, is_error) = delete_graphics( + &offline(), + json!({ + "board": board.to_str().unwrap(), + "types": ["gr_line"] + }), + ) + .await; + + assert!(is_error); + assert_eq!(body["error"]["kind"], json!("invalid_argument")); + assert!(body["message"].as_str().unwrap().contains("line, rect")); + assert_eq!(std::fs::read_to_string(&board).unwrap(), BOARD); + } + + /// A KiCad holding `board` with `shapes` (uuid, layer) on it, recording + /// every UUID a DeleteItems request asks for. + fn spawn_kicad_with_shapes( + board: &std::path::Path, + shapes: Vec<(&'static str, &'static str)>, + deleted: Arc>>, + ) -> String { + spawn_kicad_holding_board(board, move |command| { + if command.type_url.ends_with("GetItems") { + let items = shapes + .iter() + .map(|(uuid, layer)| { + let mut shape = + konnect_ipc::builders::board_segment(layer, 0.05, 0.0, 0.0, 10.0, 0.0); + shape.id = Some(kiapi::common::types::Kiid { + value: uuid.to_string(), + }); + konnect_ipc::builders::pack_any( + &shape, + "kiapi.board.types.BoardGraphicShape", + ) + }) + .collect(); + Some(konnect_ipc::builders::pack_any( + &kiapi::common::commands::GetItemsResponse { + header: None, + status: kiapi::common::types::ItemRequestStatus::IrsOk as i32, + items, + }, + "kiapi.common.commands.GetItemsResponse", + )) + } else if command.type_url.ends_with("DeleteItems") { + let delete = + kiapi::common::commands::DeleteItems::decode(command.value.as_slice()).unwrap(); + deleted + .lock() + .unwrap() + .extend(delete.item_ids.iter().map(|id| id.value.clone())); + Some(konnect_ipc::builders::pack_any( + &kiapi::common::commands::DeleteItemsResponse { + header: None, + status: kiapi::common::types::ItemRequestStatus::IrsOk as i32, + deleted_items: vec![], + }, + "kiapi.common.commands.DeleteItemsResponse", + )) + } else { + None + } + }) + } + + /// The live board wins over the file, the same way the writers in this + /// toolset act on the board KiCad holds. + #[tokio::test] + async fn a_live_board_is_edited_over_ipc_and_the_file_is_left_alone() { + let (_dir, board) = board_file(); + let deleted = Arc::new(Mutex::new(Vec::new())); + let address = spawn_kicad_with_shapes( + &board, + vec![("live-edge", "Edge.Cuts"), ("live-silk", "F.SilkS")], + deleted.clone(), + ); + let ctx = ctx_talking_to(address); + + let (body, is_error) = delete_graphics( + &ctx, + json!({ "board": board.to_str().unwrap(), "layer": "Edge.Cuts" }), + ) + .await; + + assert!(!is_error, "{body}"); + assert_eq!(body["source"], json!("ipc")); + assert_eq!(body["deleted"], json!(1)); + assert_eq!(*deleted.lock().unwrap(), vec!["live-edge".to_string()]); + // The file is the last save; KiCad owns the board, so it stays as-is. + assert_eq!(std::fs::read_to_string(&board).unwrap(), BOARD); + } + + #[tokio::test] + async fn a_filter_matching_nothing_deletes_nothing() { + let (_dir, board) = board_file(); + + let (body, is_error) = delete_graphics( + &offline(), + json!({ + "board": board.to_str().unwrap(), + "layer": "B.SilkS" + }), + ) + .await; + + assert!(!is_error, "{body}"); + assert_eq!(body["count"], json!(0)); + assert_eq!(std::fs::read_to_string(&board).unwrap(), BOARD); + } +} + #[cfg(test)] mod net_count_tests { use super::board_mock::ctx_talking_to; diff --git a/crates/konnect-ipc/src/client.rs b/crates/konnect-ipc/src/client.rs index 759547c8..267ad81d 100644 --- a/crates/konnect-ipc/src/client.rs +++ b/crates/konnect-ipc/src/client.rs @@ -216,6 +216,61 @@ fn unpack_any(any: &prost_types::Any) -> Result { M::decode(any.value.as_slice()).context("Failed to decode protobuf Any body") } +/// Decode an item only when it carries `type_name`. protobuf decoding is +/// lenient — a `BoardText` body decodes as a `BoardGraphicShape` without +/// error, yielding an item with no geometry and an empty UUID — so the +/// type_url is what says which message this is. +fn decode_as(item: &prost_types::Any, type_name: &str) -> Option { + if !item.type_url.ends_with(type_name) { + return None; + } + M::decode(item.value.as_slice()).ok() +} + +/// A KIID's textual value, or `""` when KiCad sent an item without one. +fn kiid_value(id: Option) -> String { + id.map(|id| id.value).unwrap_or_default() +} + +fn point_in_mm(point: kiapi::common::types::Vector2) -> IpcVector2 { + IpcVector2 { + x: nm_to_mm(point.x_nm), + y: nm_to_mm(point.y_nm), + } +} + +/// The normalized kind name and first defining point of a graphic shape. +fn shape_kind_and_origin( + shape: Option<&kiapi::common::types::GraphicShape>, +) -> (&'static str, Option) { + use kiapi::common::types::graphic_shape::Geometry; + use kiapi::common::types::poly_line_node::Geometry as NodeGeometry; + + let Some(geometry) = shape.and_then(|s| s.geometry.as_ref()) else { + return ("shape", None); + }; + match geometry { + Geometry::Segment(segment) => ("line", segment.start.map(point_in_mm)), + Geometry::Rectangle(rectangle) => ("rect", rectangle.top_left.map(point_in_mm)), + Geometry::Arc(arc) => ("arc", arc.start.map(point_in_mm)), + Geometry::Circle(circle) => ("circle", circle.center.map(point_in_mm)), + Geometry::Polygon(polygon) => ( + "poly", + polygon + .polygons + .first() + .and_then(|p| p.outline.as_ref()) + .and_then(|outline| outline.nodes.first()) + .and_then(|node| match node.geometry.as_ref() { + Some(NodeGeometry::Point(point)) => Some(point_in_mm(*point)), + Some(NodeGeometry::Arc(arc)) => arc.start.map(point_in_mm), + None => None, + }), + ), + Geometry::Bezier(bezier) => ("curve", bezier.start.map(point_in_mm)), + } +} + fn unpack_required( response: Option, command_name: &str, @@ -1096,6 +1151,76 @@ impl KiCadIpcClient { Ok(found) } + /// Read the board's graphics — shapes, text, textboxes, and dimensions — + /// from a specific open document. + /// + /// Reference images are not included: KiCad 10's `ReferenceImage` message + /// is an empty placeholder, so the API cannot name one, let alone identify + /// it for deletion. + pub fn get_board_graphics_in( + &self, + document: kiapi::common::types::DocumentSpecifier, + ) -> Result> { + use kiapi::board::types as board; + use kiapi::common::types::KiCadObjectType as Kot; + + let items = self.get_items_of_types_in( + document, + &[ + Kot::KotPcbShape, + Kot::KotPcbText, + Kot::KotPcbTextbox, + Kot::KotPcbDimension, + ], + )?; + + let mut graphics = Vec::new(); + for item in &items { + let graphic = if let Some(shape) = + decode_as::(item, "kiapi.board.types.BoardGraphicShape") + { + let (kind, origin) = shape_kind_and_origin(shape.shape.as_ref()); + IpcGraphic { + uuid: kiid_value(shape.id), + kind: kind.to_string(), + layer: layer_enum_to_name(shape.layer).to_string(), + origin, + } + } else if let Some(text) = + decode_as::(item, "kiapi.board.types.BoardText") + { + IpcGraphic { + uuid: kiid_value(text.id), + kind: "text".to_string(), + layer: layer_enum_to_name(text.layer).to_string(), + origin: text.text.and_then(|t| t.position).map(point_in_mm), + } + } else if let Some(textbox) = + decode_as::(item, "kiapi.board.types.BoardTextBox") + { + IpcGraphic { + uuid: kiid_value(textbox.id), + kind: "textbox".to_string(), + layer: layer_enum_to_name(textbox.layer).to_string(), + origin: textbox.textbox.and_then(|t| t.top_left).map(point_in_mm), + } + } else if let Some(dimension) = + decode_as::(item, "kiapi.board.types.Dimension") + { + IpcGraphic { + uuid: kiid_value(dimension.id), + kind: "dimension".to_string(), + layer: layer_enum_to_name(dimension.layer).to_string(), + origin: dimension.text.and_then(|t| t.position).map(point_in_mm), + } + } else { + continue; + }; + graphics.push(graphic); + } + Ok(graphics) + } + /// Read the title block of a specific open document. pub fn get_title_block_in( &self, diff --git a/crates/konnect-ipc/src/types.rs b/crates/konnect-ipc/src/types.rs index 07ff0d25..a1fb5d92 100644 --- a/crates/konnect-ipc/src/types.rs +++ b/crates/konnect-ipc/src/types.rs @@ -199,6 +199,24 @@ pub struct IpcNet { pub netcode: i32, } +/// A board graphic — a shape, text, textbox, or dimension — read back from +/// KiCad. +/// +/// `kind` is normalized (`line`, `rect`, `arc`, `circle`, `poly`, `curve`, +/// `text`, `textbox`, `dimension`) so the live and the file reader answer in +/// one vocabulary rather than protobuf names on one side and `gr_*` file tags +/// on the other. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct IpcGraphic { + pub uuid: String, + pub kind: String, + pub layer: String, + /// First defining point in mm: a segment's start, a rectangle's top-left, + /// an arc's start, a circle's centre, a polygon's first vertex, a text's + /// position. `None` when KiCad sent no geometry. + pub origin: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct IpcLayer { pub name: String, diff --git a/crates/konnect-ipc/tests/mock_server_test.rs b/crates/konnect-ipc/tests/mock_server_test.rs index 7822c4ce..7b8e3178 100644 --- a/crates/konnect-ipc/tests/mock_server_test.rs +++ b/crates/konnect-ipc/tests/mock_server_test.rs @@ -1221,3 +1221,105 @@ fn pad_reads_target_the_named_board_among_several_open() { "a read must answer about the requested board, not the first open one" ); } + +fn kiid(value: &str) -> kiapi::common::types::Kiid { + kiapi::common::types::Kiid { + value: value.to_string(), + } +} + +#[test] +fn board_graphics_come_back_with_their_kind_layer_and_identifier() { + let mut segment = builders::board_segment("Edge.Cuts", 0.05, 0.0, 0.0, 100.0, 0.0); + segment.id = Some(kiid("edge-top")); + let mut text = builders::board_text("F.SilkS", "REV A", 10.0, 20.0, 1.0, 0.0, false); + text.id = Some(kiid("silk-text")); + + // One request asks for four object types, so KiCad answers with a single + // mixed list. protobuf decoding is lenient enough to turn the text into an + // empty shape, so the reader has to dispatch on the type_url. + let mock = spawn_kicad_holding_items(vec![ + builders::pack_any(&segment, "kiapi.board.types.BoardGraphicShape"), + builders::pack_any(&text, "kiapi.board.types.BoardText"), + ]); + + let client = KiCadIpcClient::new(&mock.url); + let document = client + .find_open_board(std::path::Path::new("test.kicad_pcb")) + .expect("the mock holds test.kicad_pcb"); + + let graphics = client + .get_board_graphics_in(document) + .expect("graphics read"); + + assert_eq!(graphics.len(), 2, "the text must be read once, as text"); + assert_eq!(graphics[0].uuid, "edge-top"); + assert_eq!(graphics[0].kind, "line"); + assert_eq!(graphics[0].layer, "Edge.Cuts"); + assert_eq!( + graphics[0].origin.as_ref().map(|p| (p.x, p.y)), + Some((0.0, 0.0)) + ); + assert_eq!(graphics[1].uuid, "silk-text"); + assert_eq!(graphics[1].kind, "text"); + assert_eq!(graphics[1].layer, "F.SilkS"); + assert_eq!( + graphics[1].origin.as_ref().map(|p| (p.x, p.y)), + Some((10.0, 20.0)) + ); +} + +#[test] +fn deletes_target_the_named_board_among_several_open() { + let captured: Arc>> = Arc::new(Mutex::new(None)); + let captured_in_mock = captured.clone(); + + let mock = spawn_mock(move |request| { + let message = request.message.expect("request must pack a command"); + if message.type_url.ends_with("GetOpenDocuments") { + let response = kiapi::common::commands::GetOpenDocumentsResponse { + documents: vec![ + doc_for("other-project.kicad_pcb"), + doc_for("target.kicad_pcb"), + ], + }; + return Some(reply_with(builders::pack_any( + &response, + "kiapi.common.commands.GetOpenDocumentsResponse", + ))); + } + if message.type_url.ends_with("DeleteItems") { + let request = + kiapi::common::commands::DeleteItems::decode(message.value.as_slice()).unwrap(); + record_doc(&captured_in_mock, &request.header); + let response = kiapi::common::commands::DeleteItemsResponse { + header: None, + status: kiapi::common::types::ItemRequestStatus::IrsOk as i32, + deleted_items: vec![], + }; + return Some(reply_with(builders::pack_any( + &response, + "kiapi.common.commands.DeleteItemsResponse", + ))); + } + Some(ok_response()) + }); + + let client = KiCadIpcClient::new(&mock.url); + let document = client + .find_open_board(std::path::Path::new("target.kicad_pcb")) + .expect("target.kicad_pcb is open"); + client + .delete_items_in(document, vec!["edge-top".to_string()]) + .expect("delete"); + + let addressed = captured + .lock() + .unwrap() + .take() + .expect("the delete carried a document"); + assert_eq!( + addressed, "target.kicad_pcb", + "a delete must act on the requested board, not the first open one" + ); +} diff --git a/crates/konnect/assets/skills/kicad-pcb/SKILL.md b/crates/konnect/assets/skills/kicad-pcb/SKILL.md index 50a27d0e..a0d1ddff 100644 --- a/crates/konnect/assets/skills/kicad-pcb/SKILL.md +++ b/crates/konnect/assets/skills/kicad-pcb/SKILL.md @@ -63,7 +63,9 @@ Always call `get_active_toolsets()` first to see what is already loaded. Follow this sequence for a clean PCB workflow: -1. **Board outline** — `set_board_size` or draw Edge.Cuts geometry +1. **Board outline** — `set_board_size` or draw Edge.Cuts geometry. Both outline tools + append, so resize with `delete_graphics(layer='Edge.Cuts')` first — a second call + without it leaves two overlapping outlines and a DRC failure. 2. **Update from schematic** — call `update_pcb_from_schematic` first with `dry_run: true`. Review `status`, `coverage`, `diagnostics`, and staged positions. Apply only with `dry_run: false` and the exact returned diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 861a0f56..c54931c8 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -193,7 +193,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 222 tools from the first call. +startup, so `tools/list` carries all 223 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 2bdaeb16..4a53fce8 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 216 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 217 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, routing, library management, JLCPCB part search, Freerouting installation checks, ERC/DRC, design review audits, and full export pipelines. Tools are organized into 20 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 5557f673..39b2252a 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. 216 tools for schematic editing, PCB layout, routing, design review, and manufacturing export.", + "description": "AI-assisted PCB design via the Model Context Protocol. 217 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 02d0dab7..46e8b455 100644 --- a/tool-directory.md +++ b/tool-directory.md @@ -13,7 +13,7 @@ Compatibility notes for removed or narrowed arguments are recorded in ## Overview - **20 toolsets** organized into 10 categories -- **216 registered tools** + **6 always-visible meta-tools** = **222 total** +- **217 registered tools** + **6 always-visible meta-tools** = **223 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`. @@ -205,19 +205,20 @@ Six tools, grouped into *discovery/routing* and *observability*. ## PCB -### `pcb_board` · 11 tools +### `pcb_board` · 12 tools **Purpose:** Board outline, layers, zones, mounting holes, board text, SVG logo import. **Source:** [`crates/konnect-core/src/tools/pcb_board.rs`](crates/konnect-core/src/tools/pcb_board.rs) | Tool | Description | |------|-------------| -| `set_board_size` | Set the PCB board outline to a rectangle on the Edge.Cuts layer. | +| `set_board_size` | Add a rectangular board outline of the given dimensions on the Edge.Cuts layer. Appends — clear the old edges with `delete_graphics` first. | | `get_board_info` | Return metadata about the PCB: title, revision, company, paper size (with `paper_size_mm` dimensions on a custom User size), `layer_count`, `copper_layer_count`, and `net_count` (IPC, falls back to a file parse that counts from the tree, so KiCad 10 boards report real numbers instead of 0). | | `get_board_extents` | Return the bounding box of all objects on the board (IPC, falls back to file parse). | | `get_layer_list` | Return all layers defined in the board: `id`, `name`, `type`, plus the optional `user_name` label and a `copper` flag. | | `add_layer` | Add a new inner copper or technical layer to the board stack. Rejects a non-canonical layer name — KiCad refuses to open a board containing one. Use the canonical name and pass your own label as its user name. | | `set_active_layer` | Set the active layer recorded in the board file's setup section. | -| `add_board_outline` | Add a rectangular Edge.Cuts outline with sharp or circular rounded corners, identically over IPC and file fallback. | +| `add_board_outline` | Add a rectangular Edge.Cuts outline with sharp or circular rounded corners, identically over IPC and file fallback. Appends — clear the old edges with `delete_graphics` first. | +| `delete_graphics` | Delete board graphics (lines, rects, arcs, circles, polys, curves, text, textboxes, dimensions) matching a UUID/layer/type filter; `dry_run` lists them instead. | | `add_mounting_hole` | Add an NPTH mounting hole footprint at the specified position. | | `add_board_text` | Add a silkscreen or fabrication text string to the board. | | `add_zone` | Add a copper fill zone polygon on a specified layer and net, with optional `name`, `priority` and `pad_connection` (`solid`/`thermal`/`none`). Tries KiCad IPC first — a live board gets the zone through the API and a refill, so it appears immediately and is undoable — and falls back to an S-expression file insert only when no live KiCad answers, reporting `source` and a `warning` when it does. Refuses a net the board does not declare rather than binding copper to net 0, and refuses outright if KiCad answers but rejects the request. |