diff --git a/DEV.md b/DEV.md index 4f7015b9..270f761b 100644 --- a/DEV.md +++ b/DEV.md @@ -219,13 +219,15 @@ transaction abandon` escape hatch documented in ### Plugin Installation - **PCM zip** is the correct install method -- KiCAD installs to: `C:\Users\\Documents\KiCad\10.0\3rdparty\plugins\com_github_mixelpixx_konnect\` +- KiCad installs to: `C:\Users\\Documents\KiCad\10.0\3rdparty\plugins\com_github_mixelpixx_konnect\` - Both `__init__.py` (SWIG ActionPlugin for PCB editor settings dialog) and `plugin.json` (IPC exec plugin) are included -- `native_bridge.py` is a KiCAD-10-only, opt-in compatibility bridge. It exposes +- `native_bridge.py` is a KiCad-10-only, opt-in compatibility bridge. It exposes only authenticated status and native Specctra export over an ephemeral loopback port. The caller cannot choose an output path; the plugin owns and removes the temporary artifact. Do not grow it into a general Python RPC - surface or use it as the KiCAD 11 architecture. + surface or use it as the KiCad 11 architecture. The Rust exporter remains + the default; callers must explicitly choose `prefer` or `require` to use this + compatibility path. ## Structured Errors diff --git a/README.md b/README.md index 982a00dd..734ec8f0 100644 --- a/README.md +++ b/README.md @@ -105,8 +105,8 @@ The full tool catalog is documented in [tool-directory.md](tool-directory.md). | Layer | Mechanism | |-------|-----------| | Schematic editing | Direct `.kicad_sch` S-expression editing with atomic writes (no KiCAD required) | -| PCB editing | KiCAD 10 IPC API (NNG + protobuf) — real-time and undo-aware; single-footprint placement has a safe headless fallback | -| Specctra export | Revision-bound Rust export, with an optional authenticated KiCAD 10 ActionPlugin bridge for KiCAD-native DSN output | +| PCB editing | KiCad 10 IPC API (NNG + protobuf) — real-time and undo-aware; single-footprint placement has a safe headless fallback | +| Specctra routing | Revision-bound Rust DSN export, local Freerouting MCP routing, and strict one-transaction SES import; an authenticated KiCad 10 native-export bridge is explicit opt-in | | Exports & checks | `kicad-cli` subprocess (Gerber, PDF, ERC, DRC, …) | | Transport | MCP JSON-RPC over stdio (default), or Streamable HTTP (`transport = "http"` / `"both"`) | @@ -127,13 +127,23 @@ The full tool catalog is documented in [tool-directory.md](tool-directory.md). Verify: open the **PCB Editor** → **Tools → External Plugins** → you should see **Konnect**. -For KiCAD 10, the Konnect settings dialog also offers an optional **native +For KiCad 10, the Konnect settings dialog also offers an optional **native Specctra bridge**. When enabled, `export_specctra_dsn` can ask the active PCB Editor to generate its native DSN while Konnect still binds the export to the exact IPC snapshot and creates the strict reverse manifest used during SES import. The bridge is local-only, authenticated, disabled by default, and not -the KiCAD 11 integration path. If it is disabled or unavailable, the default -`prefer` policy falls back to Konnect's Rust exporter. +the KiCad 11 integration path. Konnect uses its Rust exporter by default; +`native_bridge_mode: "prefer"` enables fallback to Rust when the bridge is +unavailable, while `"require"` refuses instead. + +For end-to-end autorouting, run `check_freerouting`, then +`export_specctra_dsn` → `route_specctra_dsn` → +`plan_specctra_ses_import` / `apply_specctra_ses`. Readiness reports engine +discovery, native-MCP compatibility, and complete bridge availability +separately. The owned Java child is loopback-only and is reaped on success, +failure, timeout, or cancellation. The first supported profile preserves fixed +straight tracks and through vias; unlocked routing, arcs, zones, and unsupported +geometry are rejected before mutation. ### Build from source diff --git a/crates/konnect-core/src/freerouting_mcp.rs b/crates/konnect-core/src/freerouting_mcp.rs index c64935a5..c747cae6 100644 --- a/crates/konnect-core/src/freerouting_mcp.rs +++ b/crates/konnect-core/src/freerouting_mcp.rs @@ -16,6 +16,12 @@ use tokio::process::{Child, ChildStdin, ChildStdout, Command}; use tokio::task::JoinHandle; use tokio::time::{timeout, Instant}; +#[cfg(test)] +use std::sync::atomic::{AtomicU32, Ordering}; + +#[cfg(test)] +static LAST_CHILD_PID: AtomicU32 = AtomicU32::new(0); + const MCP_PROTOCOL_VERSION: &str = "2024-11-05"; const STARTUP_TIMEOUT: Duration = Duration::from_secs(30); const REQUEST_TIMEOUT: Duration = Duration::from_secs(30); @@ -23,6 +29,19 @@ const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(3); const MAX_STDERR_BYTES: usize = 64 * 1024; const MAX_SES_BYTES: u64 = 512 * 1024 * 1024; +fn server_arguments() -> &'static [&'static str] { + &[ + "--api_server.enabled=true", + "--api_server.authentication.enabled=false", + "--api_server-endpoints=http://127.0.0.1:37864", + "--mcp_server.enabled=true", + "--mcp_server.authentication.enabled=false", + "--mcp_server-endpoints=http://127.0.0.1:37964", + "--mcp_server.stdio=true", + "--gui.enabled=false", + ] +} + const REQUIRED_TOOLS: &[&str] = &[ "create_session", "enqueue_job", @@ -53,6 +72,69 @@ pub(crate) struct RouteEvidence { pub diagnostics_path: Option, } +#[derive(Debug, Clone, Serialize)] +pub(crate) struct BridgeProbe { + pub native_mcp_available: bool, + pub bridge_available: bool, + pub server_protocol_version: Option, + pub tool_count: usize, + pub diagnostics: Option, + pub error: Option, +} + +pub(crate) async fn probe_local(jar: &Path) -> BridgeProbe { + let mut client = match LocalMcpClient::start(jar).await { + Ok(client) => client, + Err(error) => { + return BridgeProbe { + native_mcp_available: false, + bridge_available: false, + server_protocol_version: None, + tool_count: 0, + diagnostics: None, + error: Some(format!("{error:#}")), + }; + } + }; + let server_protocol_version = Some(client.server_protocol_version.clone()); + let contract = async { + let tools = client.list_tools().await?; + validate_tool_contracts(&tools)?; + let missing = REQUIRED_TOOLS + .iter() + .filter(|name| !tools.contains_key(**name)) + .copied() + .collect::>(); + if !missing.is_empty() { + bail!( + "Freerouting MCP is missing required tool(s): {}", + missing.join(", ") + ); + } + Ok::(tools.len()) + } + .await; + let diagnostics = client.close().await; + match contract { + Ok(tool_count) => BridgeProbe { + native_mcp_available: true, + bridge_available: true, + server_protocol_version, + tool_count, + diagnostics: (!diagnostics.is_empty()).then_some(diagnostics), + error: None, + }, + Err(error) => BridgeProbe { + native_mcp_available: true, + bridge_available: false, + server_protocol_version, + tool_count: 0, + diagnostics: (!diagnostics.is_empty()).then_some(diagnostics), + error: Some(format!("{error:#}")), + }, + } +} + pub(crate) async fn route_local( jar: &Path, dsn: &Path, @@ -78,17 +160,18 @@ pub(crate) async fn route_local( let mut client = LocalMcpClient::start(jar).await?; let started = Instant::now(); - let route_result = timeout( + let route_result = match timeout( settings.overall_timeout, run_state_machine(&mut client, dsn, &temporary, settings, started), ) .await - .map_err(|_| { - anyhow::anyhow!( + { + Ok(result) => result, + Err(_) => Err(anyhow::anyhow!( "Freerouting MCP job exceeded the overall timeout of {} seconds", settings.overall_timeout.as_secs() - ) - })?; + )), + }; let diagnostics = client.close().await; let diagnostics_path = write_diagnostics(ses_output, &diagnostics).await?; @@ -105,6 +188,7 @@ pub(crate) async fn route_local( let evidence = match result { Ok(evidence) => evidence, Err(error) => { + let _ = tokio::fs::remove_file(&temporary).await; return Err(error); } }; @@ -183,7 +267,12 @@ fn validate_inputs( { bail!("poll interval must be between 2 and 5 seconds"); } - if settings.overall_timeout < Duration::from_secs(10) + let minimum_overall_timeout = if cfg!(test) { + Duration::from_millis(1) + } else { + Duration::from_secs(10) + }; + if settings.overall_timeout < minimum_overall_timeout || settings.overall_timeout > Duration::from_secs(86_400) { bail!("overall timeout must be between 10 and 86400 seconds"); @@ -358,28 +447,34 @@ fn require_schema_path( } struct LocalMcpClient { - child: Child, + child: Option, stdin: ChildStdin, stdout: Lines>, - stderr_task: JoinHandle, + stderr_task: Option>, next_id: u64, server_protocol_version: String, } impl LocalMcpClient { async fn start(jar: &Path) -> Result { + let mut client = Self::spawn(jar)?; + if let Err(error) = client.initialize().await { + let diagnostics = client.close().await; + return if diagnostics.is_empty() { + Err(error) + } else { + Err(error).context(format!("Freerouting stderr: {diagnostics}")) + }; + } + Ok(client) + } + + fn spawn(jar: &Path) -> Result { let mut command = Command::new("java"); command .arg("-jar") .arg(jar) - .args([ - "--api_server.enabled=true", - "--api_server.authentication.enabled=false", - "--mcp_server.enabled=true", - "--mcp_server.authentication.enabled=false", - "--mcp_server.stdio=true", - "--gui.enabled=false", - ]) + .args(server_arguments()) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -387,6 +482,10 @@ impl LocalMcpClient { let mut child = command .spawn() .context("start local Freerouting MCP server")?; + #[cfg(test)] + if let Some(pid) = child.id() { + LAST_CHILD_PID.store(pid, Ordering::SeqCst); + } let stdin = child.stdin.take().context("Freerouting MCP has no stdin")?; let stdout = child .stdout @@ -405,17 +504,20 @@ impl LocalMcpClient { .await; String::from_utf8_lossy(&bytes).trim().to_string() }); - let mut client = Self { - child, + Ok(Self { + child: Some(child), stdin, stdout: BufReader::new(stdout).lines(), - stderr_task, + stderr_task: Some(stderr_task), next_id: 1, server_protocol_version: String::new(), - }; + }) + } + + async fn initialize(&mut self) -> Result<()> { let initialize = timeout( STARTUP_TIMEOUT, - client.request( + self.request( "initialize", json!({ "protocolVersion": MCP_PROTOCOL_VERSION, @@ -427,12 +529,10 @@ impl LocalMcpClient { ) .await .map_err(|_| anyhow::anyhow!("Freerouting MCP initialization timed out"))??; - client.server_protocol_version = find_string(&initialize, &["protocolVersion"]) + self.server_protocol_version = find_string(&initialize, &["protocolVersion"]) .unwrap_or_else(|| MCP_PROTOCOL_VERSION.to_string()); - client - .notify("notifications/initialized", json!({})) - .await?; - Ok(client) + self.notify("notifications/initialized", json!({})).await?; + Ok(()) } async fn list_tools(&mut self) -> Result> { @@ -542,11 +642,32 @@ impl LocalMcpClient { async fn close(mut self) -> String { let _ = self.stdin.shutdown().await; - if timeout(SHUTDOWN_TIMEOUT, self.child.wait()).await.is_err() { - let _ = self.child.kill().await; - let _ = self.child.wait().await; + if let Some(mut child) = self.child.take() { + if timeout(SHUTDOWN_TIMEOUT, child.wait()).await.is_err() { + let _ = child.kill().await; + let _ = child.wait().await; + } + } + match self.stderr_task.take() { + Some(task) => task.await.unwrap_or_default(), + None => String::new(), + } + } +} + +impl Drop for LocalMcpClient { + fn drop(&mut self) { + if let Some(mut child) = self.child.take() { + let _ = child.start_kill(); + if let Ok(runtime) = tokio::runtime::Handle::try_current() { + runtime.spawn(async move { + let _ = child.wait().await; + }); + } + } + if let Some(task) = self.stderr_task.take() { + task.abort(); } - self.stderr_task.await.unwrap_or_default() } } @@ -643,6 +764,34 @@ mod tests { use super::*; use konnect_ipc::{IpcEffectiveRoutingRules, IpcRoutingRules}; + #[cfg(windows)] + fn process_is_running(pid: u32) -> bool { + let output = std::process::Command::new("tasklist") + .args(["/FI", &format!("PID eq {pid}"), "/FO", "CSV", "/NH"]) + .output() + .expect("run tasklist"); + String::from_utf8_lossy(&output.stdout).contains(&format!("\"{pid}\"")) + } + + #[cfg(not(windows))] + fn process_is_running(pid: u32) -> bool { + std::process::Command::new("sh") + .args(["-c", &format!("kill -0 {pid} 2>/dev/null")]) + .status() + .expect("probe process") + .success() + } + + async fn wait_for_process_exit(pid: u32) -> bool { + for _ in 0..50 { + if !process_is_running(pid) { + return true; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + false + } + fn routing_rules() -> IpcEffectiveRoutingRules { ["GND", "VCC"] .into_iter() @@ -686,6 +835,18 @@ mod tests { assert!(extension_is(&first, "ses")); } + #[test] + fn unauthenticated_servers_are_forced_to_loopback() { + let arguments = server_arguments(); + assert!(arguments.contains(&"--api_server.authentication.enabled=false")); + assert!(arguments.contains(&"--api_server-endpoints=http://127.0.0.1:37864")); + assert!(arguments.contains(&"--mcp_server.authentication.enabled=false")); + assert!(arguments.contains(&"--mcp_server-endpoints=http://127.0.0.1:37964")); + assert!(!arguments + .iter() + .any(|argument| argument.contains("0.0.0.0"))); + } + #[test] fn required_tool_schema_drift_is_refused_before_routing() { let object = |properties: Value, required: Value| json!({ "type": "object", "properties": properties, "required": required }); @@ -767,4 +928,95 @@ mod tests { assert!(evidence.ses_bytes > 0); assert!(ses.is_file()); } + + #[tokio::test] + #[ignore = "requires Java and FREEROUTING_JAR"] + async fn overall_timeout_closes_child_and_removes_partial_output() { + let jar = PathBuf::from(std::env::var_os("FREEROUTING_JAR").expect("set FREEROUTING_JAR")); + let source = include_str!("../tests/fixtures/specctra_two_resistors.kicad_pcb"); + let temp = tempfile::tempdir().unwrap(); + let board = temp.path().join("board.kicad_pcb"); + let dsn = temp.path().join("board.dsn"); + let ses = temp.path().join("board.ses"); + std::fs::write(&board, source).unwrap(); + let export = crate::specctra::export_dsn(&board, source, &routing_rules()).unwrap(); + std::fs::write(&dsn, export.dsn).unwrap(); + + let error = format!( + "{:#}", + route_local( + &jar, + &dsn, + &ses, + &RouteSettings { + max_passes: Some(100), + optimizer_enabled: Some(true), + job_timeout_seconds: Some(300), + poll_interval: Duration::from_secs(5), + overall_timeout: Duration::from_millis(100), + }, + ) + .await + .unwrap_err() + ); + assert!(error.contains("overall timeout"), "{error}"); + let pid = LAST_CHILD_PID.load(Ordering::SeqCst); + assert!( + wait_for_process_exit(pid).await, + "Freerouting child {pid} survived timeout" + ); + assert!(!ses.exists()); + assert!( + std::fs::read_dir(temp.path()).unwrap().all(|entry| !entry + .unwrap() + .file_name() + .to_string_lossy() + .contains(".tmp.ses")), + "timeout left a temporary SES" + ); + } + + #[tokio::test] + #[ignore = "requires Java and FREEROUTING_JAR"] + async fn cancelling_route_reaps_the_owned_child() { + let jar = PathBuf::from(std::env::var_os("FREEROUTING_JAR").expect("set FREEROUTING_JAR")); + let source = include_str!("../tests/fixtures/specctra_two_resistors.kicad_pcb"); + let temp = tempfile::tempdir().unwrap(); + let board = temp.path().join("board.kicad_pcb"); + let dsn = temp.path().join("board.dsn"); + let ses = temp.path().join("board.ses"); + std::fs::write(&board, source).unwrap(); + let export = crate::specctra::export_dsn(&board, source, &routing_rules()).unwrap(); + std::fs::write(&dsn, export.dsn).unwrap(); + + LAST_CHILD_PID.store(0, Ordering::SeqCst); + let task = tokio::spawn(async move { + route_local( + &jar, + &dsn, + &ses, + &RouteSettings { + max_passes: Some(100), + optimizer_enabled: Some(true), + job_timeout_seconds: Some(300), + poll_interval: Duration::from_secs(5), + overall_timeout: Duration::from_secs(300), + }, + ) + .await + }); + let pid = loop { + let pid = LAST_CHILD_PID.load(Ordering::SeqCst); + if pid != 0 { + break pid; + } + tokio::time::sleep(Duration::from_millis(50)).await; + }; + task.abort(); + let _ = task.await; + assert!( + wait_for_process_exit(pid).await, + "Freerouting child {pid} survived cancellation" + ); + } } diff --git a/crates/konnect-core/src/specctra.rs b/crates/konnect-core/src/specctra.rs index 185ae08f..7d007e79 100644 --- a/crates/konnect-core/src/specctra.rs +++ b/crates/konnect-core/src/specctra.rs @@ -68,6 +68,32 @@ struct FootprintModel { pads: Vec, } +#[derive(Debug, Clone)] +struct LockedTrackModel { + start_x_um: i64, + start_y_um: i64, + end_x_um: i64, + end_y_um: i64, + width_um: i64, + layer: String, + net: String, +} + +#[derive(Debug, Clone)] +struct LockedViaModel { + x_um: i64, + y_um: i64, + diameter_um: i64, + drill_um: i64, + net: String, +} + +#[derive(Debug, Clone, Default)] +struct LockedRouting { + tracks: Vec, + vias: Vec, +} + #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] struct RuleKey { track_width_um: i64, @@ -99,6 +125,10 @@ struct SupportedProfile { component_side: String, pad_shapes: Vec, existing_routing: bool, + #[serde(default)] + locked_track_count: usize, + #[serde(default)] + locked_via_count: usize, copper_zones: bool, custom_rules: bool, outline: String, @@ -159,10 +189,10 @@ pub(crate) fn export_dsn( bail!("KiCad IPC snapshot root is not 'kicad_pcb'"); } - reject_unsupported_board_items(&tree)?; let copper_layers = copper_layers(&tree)?; let outline = simple_closed_outline(&tree)?; let net_table = top_level_net_table(&tree); + let locked_routing = locked_routing(&tree, &net_table, &copper_layers)?; let footprints = footprints(&tree, &net_table)?; if footprints.is_empty() { bail!("supported routing profile requires at least one footprint"); @@ -218,6 +248,7 @@ pub(crate) fn export_dsn( &net_classes, &padstack_names, &via_names, + &locked_routing, )?; let dsn = serialize_and_validate(pcb)?; let source_sha256 = sha256_hex(board_source.as_bytes()); @@ -230,6 +261,7 @@ pub(crate) fn export_dsn( &net_classes, &padstack_names, &via_names, + &locked_routing, )?; Ok(ExportBundle { @@ -728,20 +760,138 @@ fn dsn_integer(node: &SexpNode, index: usize, label: &str) -> Result { Ok(value as i64) } -fn reject_unsupported_board_items(tree: &SexpNode) -> Result<()> { - let unsupported = [ - ("segment", "existing track segment"), - ("arc", "existing routed arc"), - ("via", "existing via"), - ("zone", "copper zone or rule area"), - ]; - for (tag, label) in unsupported { - let count = tree.find_all(tag).len(); - if count > 0 { - bail!("unsupported first routing profile: board contains {count} {label}(s)"); +fn locked_routing( + tree: &SexpNode, + net_table: &BTreeMap, + copper_layers: &[String], +) -> Result { + let zone_count = tree.find_all("zone").len(); + if zone_count > 0 { + bail!( + "unsupported first routing profile: board contains {zone_count} copper zone or rule area(s)" + ); + } + let arc_count = tree.find_all("arc").len(); + if arc_count > 0 { + let locked = tree + .find_all("arc") + .iter() + .filter(|arc| arc.find_str("locked") == Some("yes")) + .count(); + if locked > 0 { + bail!( + "unsupported first routing profile: board contains {locked} locked routed arc(s), which cannot be represented without approximation" + ); + } + bail!( + "unsupported first routing profile: board contains {arc_count} existing routed arc(s)" + ); + } + + let mut routing = LockedRouting::default(); + for segment in tree.find_all("segment") { + if segment.find_str("locked") != Some("yes") { + bail!("unsupported first routing profile: board contains an existing track segment that is not locked"); } + let layer = segment + .find_str("layer") + .context("locked track segment has no layer")? + .to_string(); + if !copper_layers.contains(&layer) { + bail!("locked track segment uses unsupported layer '{layer}'"); + } + let net = segment + .find("net") + .and_then(|node| resolve_net(node, net_table)) + .context("locked track segment has no connected net")?; + let (start_x_um, start_y_um) = point_um(segment, "start")?; + let (end_x_um, end_y_um) = point_um(segment, "end")?; + if start_x_um == end_x_um && start_y_um == end_y_um { + bail!("locked track segment has zero length"); + } + routing.tracks.push(LockedTrackModel { + start_x_um, + start_y_um, + end_x_um, + end_y_um, + width_um: positive_um( + segment.find("width").and_then(|width| width.get_f64(1)), + "locked track width", + )?, + layer, + net, + }); } - Ok(()) + for via in tree.find_all("via") { + if via.find_str("locked") != Some("yes") { + bail!("unsupported first routing profile: board contains an existing via that is not locked"); + } + if let Some(kind) = via.find_str("type") { + bail!("unsupported locked via type '{kind}'; only through vias are supported"); + } + let layers = via + .find("layers") + .and_then(SexpNode::children) + .unwrap_or(&[]) + .iter() + .skip(1) + .filter_map(SexpNode::as_str) + .map(str::to_string) + .collect::>(); + if layers != copper_layers { + bail!( + "unsupported locked via layer span [{}]; expected [{}]", + layers.join(", "), + copper_layers.join(", ") + ); + } + let net = via + .find("net") + .and_then(|node| resolve_net(node, net_table)) + .context("locked via has no connected net")?; + let (x_um, y_um) = point_um(via, "at")?; + let diameter_um = positive_um( + via.find("size").and_then(|size| size.get_f64(1)), + "locked via diameter", + )?; + let drill = via.find("drill").context("locked via has no drill")?; + if drill.get(1).and_then(SexpNode::as_str) == Some("oval") { + bail!("unsupported locked via with oval drill"); + } + let drill_um = positive_um(drill.get_f64(1), "locked via drill")?; + if drill_um >= diameter_um { + bail!("locked via drill is not smaller than its diameter"); + } + routing.vias.push(LockedViaModel { + x_um, + y_um, + diameter_um, + drill_um, + net, + }); + } + routing.tracks.sort_by(|left, right| { + ( + &left.net, + &left.layer, + left.start_x_um, + left.start_y_um, + left.end_x_um, + left.end_y_um, + ) + .cmp(&( + &right.net, + &right.layer, + right.start_x_um, + right.start_y_um, + right.end_x_um, + right.end_y_um, + )) + }); + routing.vias.sort_by(|left, right| { + (&left.net, left.x_um, left.y_um).cmp(&(&right.net, right.x_um, right.y_um)) + }); + Ok(routing) } fn copper_layers(tree: &SexpNode) -> Result> { @@ -1052,6 +1202,7 @@ fn build_pcb( net_classes: &BTreeMap, padstack_names: &BTreeMap, via_names: &BTreeMap, + locked_routing: &LockedRouting, ) -> Result { let default_rule = class_nets .keys() @@ -1163,6 +1314,64 @@ fn build_pcb( .collect(); debug_assert_eq!(net_classes.len(), net_pins.len()); + let net_rules = class_nets + .iter() + .flat_map(|(rule, nets)| nets.iter().map(move |net| (net, rule))) + .collect::>(); + let wires = locked_routing + .tracks + .iter() + .map(|track| { + if !net_rules.contains_key(&track.net) { + bail!("locked track uses unknown routing net '{}'", track.net); + } + Ok(dsn::Wire { + path: dsn::Path { + layer: track.layer.clone(), + width: track.width_um as f64, + coords: vec![ + dsn::Point { + x: track.start_x_um as f64, + y: track.start_y_um as f64, + }, + dsn::Point { + x: track.end_x_um as f64, + y: track.end_y_um as f64, + }, + ], + }, + net: track.net.clone(), + r#type: "fix".to_string(), + }) + }) + .collect::>>()?; + let fixed_vias = locked_routing + .vias + .iter() + .map(|via| { + let rule = net_rules + .get(&via.net) + .with_context(|| format!("locked via uses unknown routing net '{}'", via.net))?; + if via.diameter_um != rule.via_diameter_um || via.drill_um != rule.via_drill_um { + bail!( + "locked via on net '{}' is {}:{} um, but its effective via rule is {}:{} um", + via.net, + via.diameter_um, + via.drill_um, + rule.via_diameter_um, + rule.via_drill_um + ); + } + Ok(dsn::Via { + name: via_names[*rule].clone(), + x: via.x_um as f64, + y: via.y_um as f64, + net: via.net.clone(), + r#type: Some("fix".to_string()), + }) + }) + .collect::>>()?; + Ok(dsn::Pcb { name: board_path .file_name() @@ -1202,8 +1411,8 @@ fn build_pcb( library: dsn::Library { images, padstacks }, network: dsn::Network { nets, classes }, wiring: dsn::Wiring { - wires: Vec::new(), - vias: Vec::new(), + wires, + vias: fixed_vias, }, }) } @@ -1265,6 +1474,7 @@ fn build_manifest( net_classes: &BTreeMap, padstack_names: &BTreeMap, via_names: &BTreeMap, + locked_routing: &LockedRouting, ) -> Result { let components = footprints .iter() @@ -1329,7 +1539,9 @@ fn build_manifest( copper_layers: 2, component_side: "front".to_string(), pad_shapes: vec!["circle".to_string(), "rect".to_string()], - existing_routing: false, + existing_routing: !locked_routing.tracks.is_empty() || !locked_routing.vias.is_empty(), + locked_track_count: locked_routing.tracks.len(), + locked_via_count: locked_routing.vias.len(), copper_zones: false, custom_rules: false, outline: "one closed loop of straight Edge.Cuts lines".to_string(), @@ -1618,7 +1830,7 @@ mod tests { } #[test] - fn existing_routing_is_refused_before_export() { + fn unlocked_routing_is_refused_before_export() { let source = include_str!("../tests/fixtures/specctra_two_resistors.kicad_pcb").replace( "\n)", "\n (segment (start 1 1) (end 2 2) (width 0.2) (layer \"F.Cu\") (net 1))\n)", @@ -1629,6 +1841,49 @@ mod tests { assert!(error.contains("existing track segment"), "{error}"); } + #[test] + fn locked_tracks_and_vias_are_exported_as_fixed_wiring() { + let source = include_str!("../tests/fixtures/specctra_two_resistors_locked.kicad_pcb"); + let export = export_dsn(Path::new("board.kicad_pcb"), source, &rules()).unwrap(); + assert!(export.dsn.contains("(type fix)")); + assert!(export.dsn.contains("(path F.Cu 250")); + assert!(export.dsn.contains("(via konnect_via_")); + let manifest: serde_json::Value = serde_json::from_str(&export.manifest).unwrap(); + assert_eq!(manifest["supported_profile"]["existing_routing"], true); + assert_eq!(manifest["supported_profile"]["locked_track_count"], 1); + assert_eq!(manifest["supported_profile"]["locked_via_count"], 1); + } + + #[test] + fn freerouting_session_preserves_locked_routing_outside_the_import_plan() { + let source = include_str!("../tests/fixtures/specctra_two_resistors_locked.kicad_pcb"); + let native = + include_str!("../tests/fixtures/specctra_two_resistors_locked.native-kicad-10.dsn"); + let ses = + include_str!("../tests/fixtures/specctra_two_resistors_locked.freerouting-2.3.0.ses"); + let temp = tempfile::tempdir().unwrap(); + let board_path = temp.path().join("specctra_two_resistors_locked.kicad_pcb"); + std::fs::write(&board_path, source).unwrap(); + let baseline = export_dsn(&board_path, source, &native_fixture_rules()).unwrap(); + let adopted = adopt_native_dsn(baseline, native.to_string()).unwrap(); + let plan = + crate::specctra_ses::parse_import_plan(&board_path, source, &adopted.manifest, ses) + .unwrap(); + assert_eq!(plan.locked_track_count, 1); + assert_eq!(plan.locked_via_count, 1); + assert!(!plan.tracks.is_empty() || !plan.vias.is_empty()); + } + + #[test] + fn locked_arcs_are_refused_instead_of_approximated() { + let source = include_str!("../tests/fixtures/specctra_two_resistors_locked_arc.kicad_pcb"); + let error = export_dsn(Path::new("board.kicad_pcb"), source, &rules()) + .unwrap_err() + .to_string(); + assert!(error.contains("locked routed arc"), "{error}"); + assert!(error.contains("without approximation"), "{error}"); + } + #[test] fn branched_outline_is_refused() { let source = include_str!("../tests/fixtures/specctra_two_resistors.kicad_pcb") diff --git a/crates/konnect-core/src/specctra_ses.rs b/crates/konnect-core/src/specctra_ses.rs index 00b99e4f..48beeaab 100644 --- a/crates/konnect-core/src/specctra_ses.rs +++ b/crates/konnect-core/src/specctra_ses.rs @@ -18,6 +18,8 @@ pub(crate) struct SesImportPlan { pub board_path: String, pub source_sha256: String, pub session_id: String, + pub locked_track_count: usize, + pub locked_via_count: usize, pub tracks: Vec, pub arcs: Vec, pub vias: Vec, @@ -79,6 +81,10 @@ struct SupportedProfile { component_side: String, pad_shapes: Vec, existing_routing: bool, + #[serde(default)] + locked_track_count: usize, + #[serde(default)] + locked_via_count: usize, copper_zones: bool, custom_rules: bool, outline: String, @@ -156,8 +162,15 @@ pub(crate) fn parse_import_plan( .and_then(|value| value.to_str()) .context("board path has no UTF-8 file stem")?; let actual_base = atom(base_design, 1, "base_design name")?; - if actual_base != expected_base { - bail!("SES base_design '{actual_base}' does not match board '{expected_base}'"); + // Freerouting's native MCP stores the uploaded DSN under its generated job + // id and writes that id as both the session name and base_design. The exact + // target board remains bound by the manifest path and snapshot hash below; + // accept only that self-consistent transport alias, never an unrelated + // design name. + if actual_base != expected_base && actual_base != session_id { + bail!( + "SES base_design '{actual_base}' matches neither board '{expected_base}' nor session '{session_id}'" + ); } let was_is = one_child(&root, "was_is")?; @@ -192,6 +205,8 @@ pub(crate) fn parse_import_plan( board_path: manifest.board_path, source_sha256: manifest.source_sha256, session_id, + locked_track_count: manifest.supported_profile.locked_track_count, + locked_via_count: manifest.supported_profile.locked_via_count, tracks, arcs, vias, @@ -214,7 +229,6 @@ fn validate_manifest(board_path: &Path, board_source: &str, manifest: &Manifest) } if manifest.supported_profile.copper_layers != 2 || manifest.supported_profile.component_side != "front" - || manifest.supported_profile.existing_routing || manifest.supported_profile.copper_zones || manifest.supported_profile.custom_rules || manifest.supported_profile.outline.is_empty() @@ -222,6 +236,14 @@ fn validate_manifest(board_path: &Path, board_source: &str, manifest: &Manifest) { bail!("manifest does not describe the supported first routing profile"); } + let (locked_track_count, locked_via_count) = locked_routing_counts(board_source)?; + let has_locked_routing = locked_track_count > 0 || locked_via_count > 0; + if manifest.supported_profile.existing_routing != has_locked_routing + || manifest.supported_profile.locked_track_count != locked_track_count + || manifest.supported_profile.locked_via_count != locked_via_count + { + bail!("manifest locked-routing inventory does not match the live board snapshot"); + } let requested = canonical_existing(board_path)?; let recorded = canonical_existing(Path::new(&manifest.board_path))?; if requested != recorded { @@ -242,6 +264,26 @@ fn validate_manifest(board_path: &Path, board_source: &str, manifest: &Manifest) validate_manifest_relations(manifest) } +fn locked_routing_counts(board_source: &str) -> Result<(usize, usize)> { + let tree = + parse_sexp(board_source).context("parse KiCad board for locked-routing inventory")?; + if !tree.find_all("arc").is_empty() { + bail!("live board contains an unsupported routed arc"); + } + let tracks = tree.find_all("segment"); + if tracks + .iter() + .any(|track| track.find_str("locked") != Some("yes")) + { + bail!("live board contains an existing track segment that is not locked"); + } + let vias = tree.find_all("via"); + if vias.iter().any(|via| via.find_str("locked") != Some("yes")) { + bail!("live board contains an existing via that is not locked"); + } + Ok((tracks.len(), vias.len())) +} + fn validate_manifest_relations(manifest: &Manifest) -> Result<()> { let mut layer_names = BTreeSet::new(); let mut layer_indices = BTreeSet::new(); @@ -619,10 +661,15 @@ fn parse_network( let padstack = vias_by_name .get(padstack_name) .with_context(|| format!("SES via uses unknown padstack '{padstack_name}'"))?; - let nested_net = one_child(via_node, "net")?; - require_direct_shape(nested_net, 1, &[])?; - if atom(nested_net, 1, "via net")? != net_name { - bail!("SES via net does not match enclosing net '{net_name}'"); + // KiCad's SES writer repeats `(net ...)` on a via; Freerouting's + // native MCP output omits it because the via is already nested in + // `network_out/net`. Inherit only that validated enclosing net. + // When the redundant child is present, it must agree. + if let Some(nested_net) = optional_child(via_node, "net")? { + require_direct_shape(nested_net, 1, &[])?; + if atom(nested_net, 1, "via net")? != net_name { + bail!("SES via net does not match enclosing net '{net_name}'"); + } } if let Some(kind) = optional_child(via_node, "type")? { require_direct_shape(kind, 1, &[])?; @@ -875,6 +922,41 @@ mod tests { assert_eq!(plan.vias[0].drill_mm, 0.3); } + #[test] + fn native_mcp_job_alias_is_accepted_as_the_transport_design_name() { + let (_dir, board, source, manifest) = fixture(); + let ses = sample_ses() + .replacen("(session board", "(session J-ABC123", 1) + .replacen("(base_design board)", "(base_design J-ABC123)", 1); + let plan = parse_import_plan(&board, &source, &manifest, &ses).unwrap(); + assert_eq!(plan.session_id, "J-ABC123"); + } + + #[test] + fn unrelated_base_design_is_still_refused() { + let (_dir, board, source, manifest) = fixture(); + let ses = sample_ses().replacen("(base_design board)", "(base_design other)", 1); + let error = parse_import_plan(&board, &source, &manifest, &ses).unwrap_err(); + assert!(error.to_string().contains("matches neither"), "{error:#}"); + } + + #[test] + fn native_mcp_via_inherits_its_validated_enclosing_net() { + let (_dir, board, source, manifest) = fixture(); + let ses = sample_ses().replace(" (net GND) (type protect)", ""); + let plan = parse_import_plan(&board, &source, &manifest, &ses).unwrap(); + assert_eq!(plan.vias.len(), 1); + assert_eq!(plan.vias[0].net_name, "GND"); + } + + #[test] + fn redundant_via_net_must_match_its_enclosing_net() { + let (_dir, board, source, manifest) = fixture(); + let ses = sample_ses().replace("(net GND) (type protect)", "(net VCC) (type protect)"); + let error = parse_import_plan(&board, &source, &manifest, &ses).unwrap_err(); + assert!(error.to_string().contains("enclosing net"), "{error:#}"); + } + #[test] fn freerouting_owned_ses_corpus_parses() { let source = include_str!("../tests/fixtures/freerouting_issue368_no_gui_v2_3_0.ses"); diff --git a/crates/konnect-core/src/tools/integration.rs b/crates/konnect-core/src/tools/integration.rs index 0fb9f70f..3bd1534d 100644 --- a/crates/konnect-core/src/tools/integration.rs +++ b/crates/konnect-core/src/tools/integration.rs @@ -1178,6 +1178,7 @@ fn command_output(output: &std::process::Output) -> String { async fn run_java_command( command: &mut tokio::process::Command, ) -> Result { + command.kill_on_drop(true); match tokio::time::timeout(std::time::Duration::from_secs(10), command.output()).await { Ok(Ok(output)) => Ok(output), Ok(Err(error)) => Err(error.to_string()), @@ -1195,6 +1196,9 @@ async fn handle_check_freerouting( None => Ok(CallToolResult::text( serde_json::to_string(&json!({ "available": false, + "engine_found": false, + "native_mcp_available": false, + "bridge_available": false, "note": "freerouting.jar not found. Download from https://github.com/freerouting/freerouting/releases" })) .unwrap(), @@ -1207,6 +1211,9 @@ async fn handle_check_freerouting( Err(error) => { return Ok(CallToolResult::json(&json!({ "available": false, + "engine_found": true, + "native_mcp_available": false, + "bridge_available": false, "jar_path": jar_path, "java_available": false, "note": error @@ -1216,6 +1223,9 @@ async fn handle_check_freerouting( if !java.status.success() { return Ok(CallToolResult::json(&json!({ "available": false, + "engine_found": true, + "native_mcp_available": false, + "bridge_available": false, "jar_path": jar_path, "java_available": false, "java_output": command_output(&java), @@ -1231,13 +1241,21 @@ async fn handle_check_freerouting( Err(error) => (false, error), }; + let bridge = crate::freerouting_mcp::probe_local(&jar_path).await; Ok(CallToolResult::json(&json!({ "available": true, + "engine_found": true, + "native_mcp_available": bridge.native_mcp_available, + "bridge_available": bridge.bridge_available, "jar_path": jar_path, "java_available": true, "java_output": command_output(&java), "version_checked": version_checked, - "version_output": version_output + "version_output": version_output, + "server_protocol_version": bridge.server_protocol_version, + "native_mcp_tool_count": bridge.tool_count, + "native_mcp_diagnostics": bridge.diagnostics, + "bridge_error": bridge.error }))) } } @@ -1304,6 +1322,31 @@ async fn handle_route_specctra_dsn( #[cfg(test)] mod freerouting_tests { use super::*; + use crate::router::ToolRouter; + use crate::tools::ServerConfig; + use std::sync::Arc; + + fn test_ctx() -> ToolContext { + ToolContext::new( + ServerConfig { + kicad_cli: String::new(), + kicad_binary: String::new(), + ipc_address: String::new(), + project_dir: None, + jlcpcb_db_path: None, + auto_load_toolsets: false, + eager_toolsets: false, + }, + Arc::new(ToolRouter::new()), + ) + } + + fn response_json(result: &CallToolResult) -> serde_json::Value { + match &result.content[0] { + crate::mcp::protocol::ToolContent::Text { text } => serde_json::from_str(text).unwrap(), + _ => panic!("expected text content"), + } + } #[test] fn finds_versioned_jar_inside_pcm_plugin_tree() { @@ -1316,11 +1359,20 @@ mod freerouting_tests { assert_eq!(find_freerouting_jar_below(temp.path(), 5), Some(jar)); } - #[test] - fn explicit_missing_jar_does_not_claim_availability() { + #[tokio::test] + async fn explicit_missing_jar_reports_each_readiness_boundary() { let temp = tempfile::tempdir().unwrap(); let missing = temp.path().join("missing.jar"); assert_eq!(find_freerouting_jar(&json!({ "jar_path": missing })), None); + + let result = handle_check_freerouting(&json!({ "jar_path": missing }), &test_ctx()) + .await + .unwrap(); + let body = response_json(&result); + assert_eq!(body["available"], false); + assert_eq!(body["engine_found"], false); + assert_eq!(body["native_mcp_available"], false); + assert_eq!(body["bridge_available"], false); } #[test] @@ -1329,6 +1381,22 @@ mod freerouting_tests { std::fs::write(temp.path().join("other.jar"), b"fixture").unwrap(); assert_eq!(find_freerouting_jar_below(temp.path(), 5), None); } + + #[tokio::test] + #[ignore = "requires Java and FREEROUTING_JAR"] + async fn installed_engine_reports_each_observed_readiness_boundary() { + let jar = PathBuf::from(std::env::var_os("FREEROUTING_JAR").expect("set FREEROUTING_JAR")); + let result = handle_check_freerouting(&json!({ "jar_path": jar }), &test_ctx()) + .await + .unwrap(); + let body = response_json(&result); + assert_eq!(body["available"], true); + assert_eq!(body["engine_found"], true); + assert_eq!(body["native_mcp_available"], true); + assert_eq!(body["bridge_available"], true); + assert!(body["native_mcp_tool_count"].as_u64().unwrap() >= 6); + assert!(body["server_protocol_version"].as_str().is_some()); + } } #[cfg(test)] diff --git a/crates/konnect-core/src/tools/pcb_export.rs b/crates/konnect-core/src/tools/pcb_export.rs index 36c186ad..70a1a45a 100644 --- a/crates/konnect-core/src/tools/pcb_export.rs +++ b/crates/konnect-core/src/tools/pcb_export.rs @@ -295,8 +295,8 @@ pub fn tools() -> Vec { "native_bridge_mode": { "type": "string", "enum": ["prefer", "require", "disable"], - "default": "prefer", - "description": "KiCad 10 native-export policy. 'prefer' uses the enabled authenticated ActionPlugin bridge with the Rust exporter as fallback; 'require' refuses fallback; 'disable' uses Rust only." + "default": "disable", + "description": "KiCad 10 native-export policy. Rust-only 'disable' is the default; 'prefer' explicitly opts into the enabled authenticated ActionPlugin bridge with the Rust exporter as fallback; 'require' refuses fallback." } }, "required": ["board", "output"] @@ -707,6 +707,10 @@ fn write_specctra_export_pair( Ok(()) } +fn resolve_native_bridge_mode(args: &serde_json::Value) -> &str { + args["native_bridge_mode"].as_str().unwrap_or("disable") +} + async fn handle_export_specctra_dsn( args: &serde_json::Value, ctx: &ToolContext, @@ -717,7 +721,7 @@ async fn handle_export_specctra_dsn( .as_str() .map(PathBuf::from) .unwrap_or_else(|| default_specctra_manifest_path(&output_path)); - let native_bridge_mode = args["native_bridge_mode"].as_str().unwrap_or("prefer"); + let native_bridge_mode = resolve_native_bridge_mode(args); if !extension_is(&board_path, "kicad_pcb") { return Ok(invalid_export_argument( @@ -1185,6 +1189,15 @@ mod new_export_format_tests { ); } + #[test] + fn rust_specctra_export_is_the_default_path() { + assert_eq!(resolve_native_bridge_mode(&json!({})), "disable"); + assert_eq!( + resolve_native_bridge_mode(&json!({ "native_bridge_mode": "prefer" })), + "prefer" + ); + } + #[test] fn specctra_pair_writer_removes_partial_dsn_if_manifest_creation_fails() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/konnect-core/src/tools/pcb_routing.rs b/crates/konnect-core/src/tools/pcb_routing.rs index bfa83625..a99f4840 100644 --- a/crates/konnect-core/src/tools/pcb_routing.rs +++ b/crates/konnect-core/src/tools/pcb_routing.rs @@ -11,8 +11,9 @@ use crate::tools::{ }; use anyhow::Context; use konnect_sexp::writer::{apply_edits, write_atomic, SexpEdit}; +use prost::Message; use serde_json::json; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::path::Path; use super::cli; @@ -617,6 +618,10 @@ async fn handle_plan_specctra_ses_import( "track_count": plan.tracks.len(), "arc_count": plan.arcs.len(), "via_count": plan.vias.len(), + "preserved_locked_routing": { + "tracks": plan.locked_track_count, + "vias": plan.locked_via_count + }, "tracks": plan.tracks, "arcs": plan.arcs, "vias": plan.vias, @@ -642,11 +647,43 @@ struct ApplyEvidence { arc_count: usize, via_count: usize, created_count: usize, + preserved_locked_track_count: usize, + preserved_locked_via_count: usize, drc_violations: usize, unconnected_items: usize, schematic_parity_violations: usize, } +fn created_route_item_ids(items: &[prost_types::Any]) -> anyhow::Result> { + use konnect_ipc::gen::kiapi::board::types::{Arc, Track, Via}; + + let mut ids = Vec::with_capacity(items.len()); + for (index, item) in items.iter().enumerate() { + let id = if item.type_url.ends_with("kiapi.board.types.Track") { + Track::decode(item.value.as_slice())?.id + } else if item.type_url.ends_with("kiapi.board.types.Arc") { + Arc::decode(item.value.as_slice())?.id + } else if item.type_url.ends_with("kiapi.board.types.Via") { + Via::decode(item.value.as_slice())?.id + } else { + anyhow::bail!( + "KiCad returned unexpected created item type '{}' at index {index}", + item.type_url + ); + } + .with_context(|| format!("KiCad returned created route item {index} without a KIID"))? + .value; + if id.is_empty() { + anyhow::bail!("KiCad returned created route item {index} with an empty KIID"); + } + ids.push(id); + } + if ids.iter().collect::>().len() != ids.len() { + anyhow::bail!("KiCad returned duplicate KIIDs for created route items"); + } + Ok(ids) +} + async fn handle_apply_specctra_ses( args: &serde_json::Value, ctx: &ToolContext, @@ -727,9 +764,17 @@ async fn handle_apply_specctra_ses( let existing_tracks = client.get_items_in(document.clone(), ObjectType::KotPcbTrace)?; let existing_arcs = client.get_items_in(document.clone(), ObjectType::KotPcbArc)?; let existing_vias = client.get_items_in(document.clone(), ObjectType::KotPcbVia)?; - if !existing_tracks.is_empty() || !existing_arcs.is_empty() || !existing_vias.is_empty() { + if existing_tracks.len() != plan.locked_track_count + || !existing_arcs.is_empty() + || existing_vias.len() != plan.locked_via_count + { anyhow::bail!( - "live board contains routing even though the bound export profile did not" + "live locked-routing inventory changed: manifest has {} track(s)/{} via(s), IPC read {} track(s)/{} arc(s)/{} via(s)", + plan.locked_track_count, + plan.locked_via_count, + existing_tracks.len(), + existing_arcs.len(), + existing_vias.len() ); } let net_codes = client @@ -792,8 +837,7 @@ async fn handle_apply_specctra_ses( anyhow::bail!("KiCad board changed while route items were prepared; retry from a stable editor revision"); } let expected_count = items.len(); - let mut candidate_created = false; - let operation = client.run_commit("Import Freerouting SES", |client| { + let created_ids = client.run_commit("Import Freerouting SES", |client| { let created = client.create_items_in_returning(document.clone(), items)?; if created.len() != expected_count { anyhow::bail!( @@ -802,48 +846,68 @@ async fn handle_apply_specctra_ses( expected_count ); } + created_route_item_ids(&created) + })?; + + // KiCad 10 publishes neither GetItems nor SaveDocumentToString changes + // while a commit is open. End the single user-visible undo transaction, + // then validate the exact live inventory and serialized candidate. If + // any post-commit gate fails, delete only the KIIDs returned by + // CreateItems in a compensating transaction and prove the original + // serialized board was restored. + let operation = (|| { let read_tracks = client.get_items_in(document.clone(), ObjectType::KotPcbTrace)?; let read_arcs = client.get_items_in(document.clone(), ObjectType::KotPcbArc)?; let read_vias = client.get_items_in(document.clone(), ObjectType::KotPcbVia)?; - if read_tracks.len() != plan.tracks.len() + if read_tracks.len() != plan.locked_track_count + plan.tracks.len() || read_arcs.len() != plan.arcs.len() - || read_vias.len() != plan.vias.len() + || read_vias.len() != plan.locked_via_count + plan.vias.len() { anyhow::bail!( - "IPC read-back mismatch: planned {} tracks/{} arcs/{} vias, read {} tracks/{} arcs/{} vias", - plan.tracks.len(), plan.arcs.len(), plan.vias.len(), + "post-commit IPC read-back mismatch: expected {} tracks/{} arcs/{} vias, read {} tracks/{} arcs/{} vias", + plan.locked_track_count + plan.tracks.len(), + plan.arcs.len(), + plan.locked_via_count + plan.vias.len(), read_tracks.len(), read_arcs.len(), read_vias.len() ); } let candidate_source = client.save_document_to_string_in(document.clone())?; konnect_sexp::write_new_atomic(&candidate_for_ipc, &candidate_source) .with_context(|| format!("create candidate {}", candidate_for_ipc.display()))?; - candidate_created = true; let drc = runtime.block_on(cli::run_drc(&cli_path, &candidate_for_ipc, false))?; let parity_count = drc.schematic_parity.as_ref().map_or(0, Vec::len); - if !drc.violations.is_empty() || parity_count != 0 { - anyhow::bail!( - "candidate failed KiCad DRC with {} violation(s) and {} schematic-parity violation(s)", - drc.violations.len(), - parity_count - ); - } Ok(ApplyEvidence { source_sha256: plan.source_sha256.clone(), session_id: plan.session_id.clone(), - track_count: read_tracks.len(), - arc_count: read_arcs.len(), - via_count: read_vias.len(), - created_count: created.len(), + track_count: plan.tracks.len(), + arc_count: plan.arcs.len(), + via_count: plan.vias.len(), + created_count: created_ids.len(), + preserved_locked_track_count: plan.locked_track_count, + preserved_locked_via_count: plan.locked_via_count, drc_violations: drc.violations.len(), unconnected_items: drc.unconnected_items.as_ref().map_or(0, Vec::len), schematic_parity_violations: parity_count, }) - }); + })(); match operation { Ok(evidence) => Ok(evidence), Err(error) => { - if candidate_created { + let rollback = client.run_commit("Rollback failed Freerouting SES import", |client| { + client.delete_items_in(document.clone(), created_ids.clone()) + }); + if let Err(rollback_error) = rollback { + anyhow::bail!( + "SES import failed ({error}); compensating deletion also failed ({rollback_error})" + ); + } + let restored = client.save_document_to_string_in(document.clone())?; + if restored != before { + anyhow::bail!( + "SES import failed ({error}); compensating deletion completed but the live board did not return to its exact pre-import serialization" + ); + } + if candidate_for_ipc.exists() { std::fs::remove_file(&candidate_for_ipc).with_context(|| { format!( "SES import failed ({error}); also failed to remove candidate {}", @@ -879,8 +943,15 @@ async fn handle_apply_specctra_ses( "arc_count": evidence.arc_count, "via_count": evidence.via_count, "created_count": evidence.created_count, + "preserved_locked_routing": { + "tracks": evidence.preserved_locked_track_count, + "vias": evidence.preserved_locked_via_count + }, "ipc_readback": "exact_count_match", "drc": { + "clean": evidence.drc_violations == 0 + && evidence.unconnected_items == 0 + && evidence.schematic_parity_violations == 0, "violations": evidence.drc_violations, "unconnected_items": evidence.unconnected_items, "schematic_parity_violations": evidence.schematic_parity_violations @@ -1588,6 +1659,163 @@ async fn handle_route_diff_pair( }))) } +/// Manual live acceptance gate for the final #337 undo boundary. +/// +/// KiCad IPC can create a named commit but exposes no command that invokes the +/// editor's Undo action. This test therefore performs the complete import, +/// proves that routing appeared, and then waits for the operator to press +/// Ctrl+Z once in PCB Editor. It passes only when the exact pre-import IPC +/// snapshot returns. The UI action is test evidence; Konnect's runtime remains +/// IPC-only. +#[cfg(test)] +mod specctra_live_undo_test { + use super::*; + use crate::router::ToolRouter; + use crate::tools::ServerConfig; + use std::sync::Arc; + use std::time::{Duration, Instant}; + + fn response_json(result: &CallToolResult) -> serde_json::Value { + match &result.content[0] { + crate::mcp::protocol::ToolContent::Text { text } => { + serde_json::from_str(text).expect("handler returned JSON text") + } + other => panic!("expected text content, got {other:?}"), + } + } + + #[test] + fn created_route_items_must_return_unique_kicad_ids() { + use konnect_ipc::gen::kiapi::board::types::{Track, Via}; + use konnect_ipc::gen::kiapi::common::types::Kiid; + + let track = Track { + id: Some(Kiid { + value: "track-id".into(), + }), + ..Default::default() + }; + let via = Via { + id: Some(Kiid { + value: "via-id".into(), + }), + ..Default::default() + }; + let items = vec![ + konnect_ipc::builders::pack_any(&track, "kiapi.board.types.Track"), + konnect_ipc::builders::pack_any(&via, "kiapi.board.types.Via"), + ]; + assert_eq!( + created_route_item_ids(&items).unwrap(), + ["track-id", "via-id"] + ); + + let duplicate = vec![items[0].clone(), items[0].clone()]; + assert!(created_route_item_ids(&duplicate) + .unwrap_err() + .to_string() + .contains("duplicate")); + } + + #[tokio::test] + #[ignore = "requires a disposable locked fixture open in KiCad and one manual Ctrl+Z"] + async fn one_undo_restores_the_exact_pre_import_board_snapshot() { + let board = std::path::PathBuf::from( + std::env::var_os("KONNECT_LIVE_SPECCTRA_BOARD") + .expect("set KONNECT_LIVE_SPECCTRA_BOARD to the disposable open board"), + ) + .canonicalize() + .expect("resolve disposable board"); + let ipc_address = std::env::var("KICAD_API_SOCKET") + .expect("set KICAD_API_SOCKET to the PCB Editor IPC endpoint"); + let kicad_cli = std::env::var("KICAD_CLI_PATH").unwrap_or_else(|_| "kicad-cli".into()); + let freerouting_jar = std::path::PathBuf::from( + std::env::var_os("FREEROUTING_JAR").expect("set FREEROUTING_JAR"), + ); + let client = konnect_ipc::KiCadIpcClient::new(&ipc_address); + let document = client.find_open_board(&board).expect("find open board"); + let before = client + .save_document_to_string_in(document.clone()) + .expect("capture board before import"); + let rules = client + .get_effective_routing_rules_in(document.clone()) + .expect("capture routing rules"); + let export = crate::specctra::export_dsn(&board, &before, &rules) + .expect("export locked-routing fixture"); + + let temp = tempfile::tempdir().expect("create output directory"); + let dsn = temp.path().join("board.dsn"); + let manifest = temp.path().join("board.dsn.konnect.json"); + let ses = temp.path().join("board.ses"); + let candidate = temp.path().join("board.freerouted.kicad_pcb"); + std::fs::write(&dsn, export.dsn).expect("write deterministic DSN"); + std::fs::write(&manifest, export.manifest).expect("write reverse manifest"); + crate::freerouting_mcp::route_local( + &freerouting_jar, + &dsn, + &ses, + &crate::freerouting_mcp::RouteSettings { + max_passes: Some(20), + optimizer_enabled: Some(false), + job_timeout_seconds: Some(120), + poll_interval: Duration::from_secs(2), + overall_timeout: Duration::from_secs(180), + }, + ) + .await + .expect("route deterministic DSN through local Freerouting MCP"); + let ctx = ToolContext::new( + ServerConfig { + kicad_cli, + kicad_binary: String::new(), + ipc_address: ipc_address.clone(), + project_dir: None, + jlcpcb_db_path: None, + auto_load_toolsets: false, + eager_toolsets: false, + }, + Arc::new(ToolRouter::new()), + ); + let result = handle_apply_specctra_ses( + &json!({ + "board": board, + "ses_path": ses, + "manifest_path": manifest, + "candidate_output_path": candidate + }), + &ctx, + ) + .await + .expect("apply handler returned"); + assert!(!result.is_error, "{}", response_json(&result)); + let body = response_json(&result); + assert_eq!(body["success"], true); + assert_eq!(body["undo_description"], "Import Freerouting SES"); + assert!(body["created_count"].as_u64().unwrap_or(0) > 0); + + let after = client + .save_document_to_string_in(document.clone()) + .expect("capture board after import"); + assert_ne!(after, before, "import created no observable board change"); + eprintln!("LIVE_UNDO_READY: press Ctrl+Z once in PCB Editor"); + + let deadline = Instant::now() + Duration::from_secs(60); + loop { + let observed = client + .save_document_to_string_in(document.clone()) + .expect("observe board while waiting for undo"); + if observed == before { + break; + } + assert!( + Instant::now() < deadline, + "one Ctrl+Z did not restore the exact pre-import IPC snapshot within 60 seconds" + ); + std::thread::sleep(Duration::from_millis(200)); + } + } +} + #[cfg(test)] mod add_net_format_tests { use super::*; diff --git a/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.README.md b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.README.md new file mode 100644 index 00000000..a770f080 --- /dev/null +++ b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.README.md @@ -0,0 +1,18 @@ +# Locked-routing Specctra fixture + +`specctra_two_resistors_locked.kicad_pcb` was produced by loading +`specctra_two_resistors.kicad_pcb` through KiCad 10.0.5's `pcbnew` API, adding +one locked `PCB_TRACK` and one locked through `PCB_VIA`, and saving through +KiCad. Its matching `.native-kicad-10.dsn` file was then produced by KiCad +10.0.5's native `ExportSpecctraDSN` implementation. The matching +`.freerouting-2.3.0.ses` was produced by routing that DSN through Freerouting +2.3.0 with two passes. The native DSN's identifying `pcb` atom was normalized +from the generation-machine output path to the stable fixture filename; no +routing semantics were changed. + +The `_locked_arc` pair was produced the same way with one additional locked +`PCB_ARC`. These native files are parity oracles, not runtime dependencies. The +exports prove that KiCad represents locked straight tracks and vias as +Specctra `type fix`, while lowering the locked arc to its straight start/end +chord. Konnect's fail-closed Rust profile therefore rejects locked arcs rather +than silently approximating their geometry. diff --git a/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.freerouting-2.3.0.ses b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.freerouting-2.3.0.ses new file mode 100644 index 00000000..be77cf1e --- /dev/null +++ b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.freerouting-2.3.0.ses @@ -0,0 +1,62 @@ +(session "specctra_two_resistors_locked" + (base_design "specctra_two_resistors_locked") + (placement + (resolution um 10) + (component "Resistor_SMD:R_0402" + (place R1 1000000 -500000 front 0) + (place R2 1100000 -500000 front 0) + ) + ) + (was_is + ) + (routes + (resolution um 10) + (parser + (host_cad "KiCad's Pcbnew") + (host_version "10.0.5") + ) + (library_out + (padstack "Via[0-1]_600:300_um" + (shape + (circle F.Cu 6000 0 0) + ) + (shape + (circle B.Cu 6000 0 0) + ) + (attach off) + ) + ) + (network_out + (net VCC + (wire + (path F.Cu 2000 + 1105000 -505517 + 1104455 -506062 + 995545 -506062 + 995000 -505517 + ) + ) + (wire + (path F.Cu 2000 + 1105000 -500000 + 1105000 -505517 + ) + ) + (wire + (path F.Cu 2000 + 995000 -500000 + 995000 -505517 + ) + ) + ) + (net GND + (wire + (path F.Cu 2000 + 1050000 -500000 + 1095000 -500000 + ) + ) + ) + ) + ) +) \ No newline at end of file diff --git a/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.kicad_pcb b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.kicad_pcb new file mode 100644 index 00000000..45b7dc8a --- /dev/null +++ b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.kicad_pcb @@ -0,0 +1,275 @@ +(kicad_pcb + (version 20260206) + (generator "pcbnew") + (generator_version "10.0") + (general + (thickness 1.6) + (legacy_teardrops no) + ) + (paper "A4") + (title_block + (title "Test Board") + (date "2024-01-01") + (rev "1.0") + (company "Test Co") + ) + (layers + (0 "F.Cu" signal) + (2 "B.Cu" signal) + (9 "F.Adhes" user "F.Adhesive") + (11 "B.Adhes" user "B.Adhesive") + (13 "F.Paste" user) + (15 "B.Paste" user) + (5 "F.SilkS" user "F.Silkscreen") + (7 "B.SilkS" user "B.Silkscreen") + (1 "F.Mask" user) + (3 "B.Mask" user) + (25 "Edge.Cuts" user) + (27 "Margin" user) + (31 "F.CrtYd" user "F.Courtyard") + (29 "B.CrtYd" user "B.Courtyard") + ) + (setup + (pad_to_mask_clearance 0.05) + (allow_soldermask_bridges_in_footprints no) + (tenting + (front yes) + (back yes) + ) + (covering + (front no) + (back no) + ) + (plugging + (front no) + (back no) + ) + (capping no) + (filling no) + (pcbplotparams + (layerselection 0x00000000_00000000_55555555_5755f5ff) + (plot_on_all_layers_selection 0x00000000_00000000_00000000_00000000) + (disableapertmacros no) + (usegerberextensions no) + (usegerberattributes yes) + (usegerberadvancedattributes yes) + (creategerberjobfile yes) + (dashed_line_dash_ratio 12) + (dashed_line_gap_ratio 3) + (svgprecision 4) + (plotframeref no) + (mode 1) + (useauxorigin no) + (pdf_front_fp_property_popups yes) + (pdf_back_fp_property_popups yes) + (pdf_metadata yes) + (pdf_single_document no) + (dxfpolygonmode yes) + (dxfimperialunits yes) + (dxfusepcbnewfont yes) + (psnegative no) + (psa4output no) + (plot_black_and_white yes) + (sketchpadsonfab no) + (plotpadnumbers no) + (hidednponfab no) + (sketchdnponfab yes) + (crossoutdnponfab yes) + (subtractmaskfromsilk no) + (outputformat 1) + (mirror no) + (drillshape 1) + (scaleselection 1) + (outputdirectory "") + ) + ) + (footprint "Resistor_SMD:R_0402" + (layer "F.Cu") + (uuid "11111111-1111-4111-8111-111111111111") + (at 100 50) + (property "Reference" "R1" + (at 0 -1.5 0) + (layer "F.SilkS") + (uuid "0c60d4fc-5d01-457d-942e-5dc4c5d7f73c") + (effects + (font + (size 1 1) + (thickness 0.15) + ) + ) + ) + (property "Value" "10k" + (at 0 1.5 0) + (layer "F.Fab") + (uuid "330d0924-3a6c-4931-989b-6dda45421585") + (effects + (font + (size 1 1) + (thickness 0.15) + ) + ) + ) + (property "Datasheet" "" + (at 0 0 0) + (layer "F.Fab") + (hide yes) + (uuid "39a6229d-eb58-4939-bad2-d0ff201f0750") + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Description" "" + (at 0 0 0) + (layer "F.Fab") + (hide yes) + (uuid "764436b9-58d7-42ef-9e9a-11e88174a533") + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (duplicate_pad_numbers_are_jumpers no) + (pad "1" smd rect + (at -0.5 0) + (size 0.6 0.5) + (layers "F.Cu" "F.Mask" "F.Paste") + (net "VCC") + (uuid "fb481b08-0972-41e5-a5d9-a4bab9f165c1") + ) + (pad "2" smd rect + (at 0.5 0) + (size 0.6 0.5) + (layers "F.Cu" "F.Mask" "F.Paste") + (net "GND") + (uuid "80cbeaee-444b-4a13-95c2-2c277db1ac17") + ) + (embedded_fonts no) + ) + (footprint "Resistor_SMD:R_0402" + (layer "F.Cu") + (uuid "22222222-2222-4222-8222-222222222222") + (at 110 50) + (property "Reference" "R2" + (at 0 -1.5 0) + (layer "F.SilkS") + (uuid "be2dcc12-71f8-4040-aeb6-d692cc821f6a") + (effects + (font + (size 1 1) + (thickness 0.15) + ) + ) + ) + (property "Value" "4.7k" + (at 0 1.5 0) + (layer "F.Fab") + (uuid "4d4da4d3-8e8a-4449-9f17-1776fc32f226") + (effects + (font + (size 1 1) + (thickness 0.15) + ) + ) + ) + (property "Datasheet" "" + (at 0 0 0) + (layer "F.Fab") + (hide yes) + (uuid "c841b0c2-7eec-4bce-a5ee-32f302f9e018") + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Description" "" + (at 0 0 0) + (layer "F.Fab") + (hide yes) + (uuid "fcbdc275-d4f9-49f0-b054-feec209526e4") + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (duplicate_pad_numbers_are_jumpers no) + (pad "1" smd rect + (at -0.5 0) + (size 0.6 0.5) + (layers "F.Cu" "F.Mask" "F.Paste") + (net "GND") + (uuid "611a5e55-7fda-4eb1-9177-2b7af8a58dba") + ) + (pad "2" smd rect + (at 0.5 0) + (size 0.6 0.5) + (layers "F.Cu" "F.Mask" "F.Paste") + (net "VCC") + (uuid "10e20b3d-431e-4a39-a976-77389855efa1") + ) + (embedded_fonts no) + ) + (gr_line + (start 80 30) + (end 130 30) + (stroke + (width 0.05) + (type default) + ) + (layer "Edge.Cuts") + (uuid "33333333-3333-4333-8333-333333333331") + ) + (gr_line + (start 80 70) + (end 80 30) + (stroke + (width 0.05) + (type default) + ) + (layer "Edge.Cuts") + (uuid "33333333-3333-4333-8333-333333333334") + ) + (gr_line + (start 130 30) + (end 130 70) + (stroke + (width 0.05) + (type default) + ) + (layer "Edge.Cuts") + (uuid "33333333-3333-4333-8333-333333333332") + ) + (gr_line + (start 130 70) + (end 80 70) + (stroke + (width 0.05) + (type default) + ) + (layer "Edge.Cuts") + (uuid "33333333-3333-4333-8333-333333333333") + ) + (segment + (start 100.5 50) + (end 105 50) + (width 0.25) + (locked yes) + (layer "F.Cu") + (net "GND") + (uuid "cb3de6bf-0cdb-4170-b26a-66acb0373995") + ) + (via + (at 105 50) + (size 0.6) + (drill 0.3) + (layers "F.Cu" "B.Cu") + (locked yes) + (net "GND") + (uuid "db24f42e-63bc-4e53-a1a7-509928ef3cd6") + ) + (embedded_fonts no) +) diff --git a/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.native-kicad-10.dsn b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.native-kicad-10.dsn new file mode 100644 index 00000000..5c654092 --- /dev/null +++ b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.native-kicad-10.dsn @@ -0,0 +1,76 @@ +(pcb "specctra_two_resistors_locked.native-kicad-10.dsn" + (parser + (string_quote ") + (space_in_quoted_tokens on) + (host_cad "KiCad's Pcbnew") + (host_version "10.0.5") + ) + (resolution um 10) + (unit um) + (structure + (layer F.Cu + (type signal) + (property + (index 0) + ) + ) + (layer B.Cu + (type signal) + (property + (index 1) + ) + ) + (boundary + (path pcb 0 130000 -70000 80000 -70000 80000 -30000 130000 -30000 + 130000 -70000) + ) + (via "Via[0-1]_600:300_um") + (rule + (width 200) + (clearance 200) + (clearance 50 (type smd_smd)) + ) + ) + (placement + (component Resistor_SMD:R_0402 + (place R1 100000 -50000 front 0 (PN 10k)) + (place R2 110000 -50000 front 0 (PN 4.7k)) + ) + ) + (library + (image Resistor_SMD:R_0402 + (pin Rect[T]Pad_600.000000x500.000000_um 1 -500 0) + (pin Rect[T]Pad_600.000000x500.000000_um 2 500 0) + ) + (padstack Rect[T]Pad_600.000000x500.000000_um + (shape (rect F.Cu -300 -250 300 250)) + (attach off) + ) + (padstack "Via[0-1]_600:300_um" + (shape (circle F.Cu 600)) + (shape (circle B.Cu 600)) + (attach off) + ) + ) + (network + (net VCC + (pins R1-1 R2-2) + ) + (net GND + (pins R1-2 R2-1) + ) + (class kicad_default GND VCC + (circuit + (use_via "Via[0-1]_600:300_um") + ) + (rule + (width 200) + (clearance 200) + ) + ) + ) + (wiring + (wire (path F.Cu 250 100500 -50000 105000 -50000)(net GND)(type fix)) + (via "Via[0-1]_600:300_um" 105000 -50000 (net GND)(type fix)) + ) +) diff --git a/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked_arc.kicad_pcb b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked_arc.kicad_pcb new file mode 100644 index 00000000..0eba642b --- /dev/null +++ b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked_arc.kicad_pcb @@ -0,0 +1,285 @@ +(kicad_pcb + (version 20260206) + (generator "pcbnew") + (generator_version "10.0") + (general + (thickness 1.6) + (legacy_teardrops no) + ) + (paper "A4") + (title_block + (title "Test Board") + (date "2024-01-01") + (rev "1.0") + (company "Test Co") + ) + (layers + (0 "F.Cu" signal) + (2 "B.Cu" signal) + (9 "F.Adhes" user "F.Adhesive") + (11 "B.Adhes" user "B.Adhesive") + (13 "F.Paste" user) + (15 "B.Paste" user) + (5 "F.SilkS" user "F.Silkscreen") + (7 "B.SilkS" user "B.Silkscreen") + (1 "F.Mask" user) + (3 "B.Mask" user) + (25 "Edge.Cuts" user) + (27 "Margin" user) + (31 "F.CrtYd" user "F.Courtyard") + (29 "B.CrtYd" user "B.Courtyard") + ) + (setup + (pad_to_mask_clearance 0.05) + (allow_soldermask_bridges_in_footprints no) + (tenting + (front yes) + (back yes) + ) + (covering + (front no) + (back no) + ) + (plugging + (front no) + (back no) + ) + (capping no) + (filling no) + (pcbplotparams + (layerselection 0x00000000_00000000_55555555_5755f5ff) + (plot_on_all_layers_selection 0x00000000_00000000_00000000_00000000) + (disableapertmacros no) + (usegerberextensions no) + (usegerberattributes yes) + (usegerberadvancedattributes yes) + (creategerberjobfile yes) + (dashed_line_dash_ratio 12) + (dashed_line_gap_ratio 3) + (svgprecision 4) + (plotframeref no) + (mode 1) + (useauxorigin no) + (pdf_front_fp_property_popups yes) + (pdf_back_fp_property_popups yes) + (pdf_metadata yes) + (pdf_single_document no) + (dxfpolygonmode yes) + (dxfimperialunits yes) + (dxfusepcbnewfont yes) + (psnegative no) + (psa4output no) + (plot_black_and_white yes) + (sketchpadsonfab no) + (plotpadnumbers no) + (hidednponfab no) + (sketchdnponfab yes) + (crossoutdnponfab yes) + (subtractmaskfromsilk no) + (outputformat 1) + (mirror no) + (drillshape 1) + (scaleselection 1) + (outputdirectory "") + ) + ) + (footprint "Resistor_SMD:R_0402" + (layer "F.Cu") + (uuid "11111111-1111-4111-8111-111111111111") + (at 100 50) + (property "Reference" "R1" + (at 0 -1.5 0) + (layer "F.SilkS") + (uuid "0c60d4fc-5d01-457d-942e-5dc4c5d7f73c") + (effects + (font + (size 1 1) + (thickness 0.15) + ) + ) + ) + (property "Value" "10k" + (at 0 1.5 0) + (layer "F.Fab") + (uuid "330d0924-3a6c-4931-989b-6dda45421585") + (effects + (font + (size 1 1) + (thickness 0.15) + ) + ) + ) + (property "Datasheet" "" + (at 0 0 0) + (layer "F.Fab") + (hide yes) + (uuid "39a6229d-eb58-4939-bad2-d0ff201f0750") + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Description" "" + (at 0 0 0) + (layer "F.Fab") + (hide yes) + (uuid "764436b9-58d7-42ef-9e9a-11e88174a533") + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (duplicate_pad_numbers_are_jumpers no) + (pad "1" smd rect + (at -0.5 0) + (size 0.6 0.5) + (layers "F.Cu" "F.Mask" "F.Paste") + (net "VCC") + (uuid "fb481b08-0972-41e5-a5d9-a4bab9f165c1") + ) + (pad "2" smd rect + (at 0.5 0) + (size 0.6 0.5) + (layers "F.Cu" "F.Mask" "F.Paste") + (net "GND") + (uuid "80cbeaee-444b-4a13-95c2-2c277db1ac17") + ) + (embedded_fonts no) + ) + (footprint "Resistor_SMD:R_0402" + (layer "F.Cu") + (uuid "22222222-2222-4222-8222-222222222222") + (at 110 50) + (property "Reference" "R2" + (at 0 -1.5 0) + (layer "F.SilkS") + (uuid "be2dcc12-71f8-4040-aeb6-d692cc821f6a") + (effects + (font + (size 1 1) + (thickness 0.15) + ) + ) + ) + (property "Value" "4.7k" + (at 0 1.5 0) + (layer "F.Fab") + (uuid "4d4da4d3-8e8a-4449-9f17-1776fc32f226") + (effects + (font + (size 1 1) + (thickness 0.15) + ) + ) + ) + (property "Datasheet" "" + (at 0 0 0) + (layer "F.Fab") + (hide yes) + (uuid "c841b0c2-7eec-4bce-a5ee-32f302f9e018") + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Description" "" + (at 0 0 0) + (layer "F.Fab") + (hide yes) + (uuid "fcbdc275-d4f9-49f0-b054-feec209526e4") + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (duplicate_pad_numbers_are_jumpers no) + (pad "1" smd rect + (at -0.5 0) + (size 0.6 0.5) + (layers "F.Cu" "F.Mask" "F.Paste") + (net "GND") + (uuid "611a5e55-7fda-4eb1-9177-2b7af8a58dba") + ) + (pad "2" smd rect + (at 0.5 0) + (size 0.6 0.5) + (layers "F.Cu" "F.Mask" "F.Paste") + (net "VCC") + (uuid "10e20b3d-431e-4a39-a976-77389855efa1") + ) + (embedded_fonts no) + ) + (gr_line + (start 80 30) + (end 130 30) + (stroke + (width 0.05) + (type default) + ) + (layer "Edge.Cuts") + (uuid "33333333-3333-4333-8333-333333333331") + ) + (gr_line + (start 80 70) + (end 80 30) + (stroke + (width 0.05) + (type default) + ) + (layer "Edge.Cuts") + (uuid "33333333-3333-4333-8333-333333333334") + ) + (gr_line + (start 130 30) + (end 130 70) + (stroke + (width 0.05) + (type default) + ) + (layer "Edge.Cuts") + (uuid "33333333-3333-4333-8333-333333333332") + ) + (gr_line + (start 130 70) + (end 80 70) + (stroke + (width 0.05) + (type default) + ) + (layer "Edge.Cuts") + (uuid "33333333-3333-4333-8333-333333333333") + ) + (arc + (start 99.5 50) + (mid 97.25 47.75) + (end 95 50) + (width 0.25) + (locked yes) + (layer "F.Cu") + (net "VCC") + (uuid "ebe43b04-2de9-4c31-a95c-41d91ac54fd9") + ) + (segment + (start 100.5 50) + (end 105 50) + (width 0.25) + (locked yes) + (layer "F.Cu") + (net "GND") + (uuid "7c882cfd-586c-42f2-b9a8-c05b47169593") + ) + (via + (at 105 50) + (size 0.6) + (drill 0.3) + (layers "F.Cu" "B.Cu") + (locked yes) + (net "GND") + (uuid "172c4b75-d171-477c-98c8-ba3ad78ceeee") + ) + (embedded_fonts no) +) diff --git a/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked_arc.native-kicad-10.dsn b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked_arc.native-kicad-10.dsn new file mode 100644 index 00000000..fec99b3e --- /dev/null +++ b/crates/konnect-core/tests/fixtures/specctra_two_resistors_locked_arc.native-kicad-10.dsn @@ -0,0 +1,77 @@ +(pcb "specctra_two_resistors_locked_arc.native-kicad-10.dsn" + (parser + (string_quote ") + (space_in_quoted_tokens on) + (host_cad "KiCad's Pcbnew") + (host_version "10.0.5") + ) + (resolution um 10) + (unit um) + (structure + (layer F.Cu + (type signal) + (property + (index 0) + ) + ) + (layer B.Cu + (type signal) + (property + (index 1) + ) + ) + (boundary + (path pcb 0 130000 -70000 80000 -70000 80000 -30000 130000 -30000 + 130000 -70000) + ) + (via "Via[0-1]_600:300_um") + (rule + (width 200) + (clearance 200) + (clearance 50 (type smd_smd)) + ) + ) + (placement + (component Resistor_SMD:R_0402 + (place R1 100000 -50000 front 0 (PN 10k)) + (place R2 110000 -50000 front 0 (PN 4.7k)) + ) + ) + (library + (image Resistor_SMD:R_0402 + (pin Rect[T]Pad_600.000000x500.000000_um 1 -500 0) + (pin Rect[T]Pad_600.000000x500.000000_um 2 500 0) + ) + (padstack Rect[T]Pad_600.000000x500.000000_um + (shape (rect F.Cu -300 -250 300 250)) + (attach off) + ) + (padstack "Via[0-1]_600:300_um" + (shape (circle F.Cu 600)) + (shape (circle B.Cu 600)) + (attach off) + ) + ) + (network + (net VCC + (pins R1-1 R2-2) + ) + (net GND + (pins R1-2 R2-1) + ) + (class kicad_default GND VCC + (circuit + (use_via "Via[0-1]_600:300_um") + ) + (rule + (width 200) + (clearance 200) + ) + ) + ) + (wiring + (wire (path F.Cu 250 99500 -50000 95000 -50000)(net VCC)(type fix)) + (wire (path F.Cu 250 100500 -50000 105000 -50000)(net GND)(type fix)) + (via "Via[0-1]_600:300_um" 105000 -50000 (net GND)(type fix)) + ) +) diff --git a/docs/API_MIGRATIONS.md b/docs/API_MIGRATIONS.md index aeeed6b5..6d834e27 100644 --- a/docs/API_MIGRATIONS.md +++ b/docs/API_MIGRATIONS.md @@ -3,6 +3,17 @@ Konnect's tool schemas are public API. This file records intentional argument removals and the supported replacement workflow. +## Unreleased: Rust Specctra export is the default + +`export_specctra_dsn.native_bridge_mode` now defaults to `disable`, so an +omitted value always selects the Rust/IPC exporter. This keeps the default path +free of Python and SWIG and makes its KiCad 11 direction explicit. + +KiCad 10 users who deliberately want the authenticated ActionPlugin bridge can +pass `prefer` (use the native export when available, otherwise Rust) or +`require` (refuse when the native bridge is unavailable). No tool or argument +was removed. + ## Unreleased: remove inputs that never affected an operation The following optional inputs were advertised but never read by their handlers. diff --git a/docs/TESTING_AND_RELEASE.md b/docs/TESTING_AND_RELEASE.md index 31bf63d8..8a9d72db 100644 --- a/docs/TESTING_AND_RELEASE.md +++ b/docs/TESTING_AND_RELEASE.md @@ -49,6 +49,21 @@ tests should prove request construction and failure classification; an ignored live test or the end-to-end workflow should prove behavior that depends on a running editor. +The Specctra import undo boundary has a manual live gate because KiCad IPC can +create a named commit but cannot invoke the editor's Undo action. Open a +disposable copy of +`crates/konnect-core/tests/fixtures/specctra_two_resistors_locked.kicad_pcb`, +set `KONNECT_LIVE_SPECCTRA_BOARD`, `KICAD_API_SOCKET`, and (when it is not on +`PATH`) `KICAD_CLI_PATH`, set `FREEROUTING_JAR` to the local Freerouting 2.3.0 +JAR, then run: + +```text +cargo test -p konnect-core --locked one_undo_restores_the_exact_pre_import_board_snapshot -- --ignored --nocapture +``` + +When the test prints `LIVE_UNDO_READY`, press Ctrl+Z once in PCB Editor. The +test passes only when the exact pre-import IPC snapshot returns. + ## CI And Live Validation `.github/workflows/ci.yml` covers the Rust workspace, formatting, clippy, diff --git a/tool-directory.md b/tool-directory.md index a2a23957..29fc7fc5 100644 --- a/tool-directory.md +++ b/tool-directory.md @@ -260,8 +260,8 @@ Six tools, grouped into *discovery/routing* and *observability*. | `route_trace` | Route a trace segment between two points on a copper layer via KiCAD IPC. | | `route_pad_to_pad` | Route a direct trace between two pads of named components (L-bend routing) via IPC. | | `add_via` | Add a through-hole via at a position and assign it to a net via IPC. | -| `plan_specctra_ses_import` | Strictly validate a Freerouting SES against its revision-bound manifest and the exact live board, returning every planned track and via without mutation. | -| `apply_specctra_ses` | Apply a validated SES through KiCad IPC as one undo transaction, create a separate candidate board, verify IPC read-back, and run KiCad DRC before commit. | +| `plan_specctra_ses_import` | Strictly validate a Freerouting SES against its revision-bound manifest and the exact live board, returning every planned route item and the preserved locked-track/via inventory without mutation. | +| `apply_specctra_ses` | Preserve the manifest-bound locked straight tracks and through vias, apply a validated SES through KiCad IPC as one undo transaction, verify post-commit IPC read-back, create a separate candidate board, and report direct KiCad DRC evidence (including whether it is clean). | | `add_copper_pour` | Alias of `add_zone`, kept for compatibility: same arguments, same defaults, same IPC-first behaviour. (Its `min_width` default was 0.25 and is now 0.2, matching `add_zone` and KiCad.) | | `delete_trace` | Delete a trace segment identified by its UUID via KiCAD IPC. | | `query_traces` | List trace segments on the board, optionally filtered by net and/or layer. | @@ -298,7 +298,7 @@ Six tools, grouped into *discovery/routing* and *observability*. | `export_3d` | Export the PCB as a 3D model using kicad-cli, with explicit control over unspecified footprint models. | | `export_bom` | Generate KiCad 10's CSV Bill of Materials from schematic fields. | | `export_netlist` | Export the PCB netlist in KiCAD or IPC-D-356 format. | -| `export_specctra_dsn` | Export a deterministic, revision-bound Specctra DSN plus reverse manifest from a supported live KiCad board. On KiCad 10, `native_bridge_mode` can prefer or require the optional authenticated ActionPlugin native exporter; otherwise Konnect uses its Rust exporter. Refuses unsupported geometry or incomplete rules. | +| `export_specctra_dsn` | Export a deterministic, revision-bound Specctra DSN plus reverse manifest from a supported live KiCad board. The Rust exporter is the default. On KiCad 10, explicitly set `native_bridge_mode` to `prefer` or `require` for the optional authenticated ActionPlugin native exporter. Preserves locked straight tracks and through vias as fixed wiring; refuses unlocked routing, arcs, unsupported geometry, or incomplete rules. | | `export_position_file` | Generate a component placement (pick-and-place) position file for SMT assembly. | | `export_dxf` | Export the PCB to DXF, one file per requested layer, using kicad-cli. `layers` is required — there is no all-layers default. For mechanical CAD interchange. | | `export_gencad` | Export the PCB in GenCAD format using kicad-cli. | @@ -352,8 +352,8 @@ Six tools, grouped into *discovery/routing* and *observability*. | `get_jlcpcb_database_stats` | Statistics about the local JLCPCB cache: part count, last updated, file size. | | `enrich_datasheets` | Fetch and cache datasheet URLs for all components in a schematic (LCSC API). | | `get_datasheet_url` | Retrieve the datasheet URL for a component by MPN or LCSC ID — from the local JLCPCB catalog first, falling back to the LCSC API. | -| `check_freerouting` | Locate a Freerouting installation, including KiCad PCM plugin directories, and verify that its Java runtime is available. | -| `route_specctra_dsn` | Route a DSN through the discovered local Freerouting JAR's native headless MCP server and create a new SES without cloud upload or replacement. | +| `check_freerouting` | Locate a Freerouting installation, including KiCad PCM plugin directories, then report `engine_found`, `native_mcp_available`, and `bridge_available` as separate observed facts. | +| `route_specctra_dsn` | Route a DSN through the discovered local Freerouting JAR's native headless MCP server and create a new SES without cloud upload or replacement. The owned unauthenticated service is loopback-only; its child is reaped on every exit path. | Migration from the former `autoroute` tool: use `export_specctra_dsn`, `route_specctra_dsn`, then `plan_specctra_ses_import` / `apply_specctra_ses`.