diff --git a/AGENTS.md b/AGENTS.md index aafd055..532ef01 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -246,6 +246,12 @@ Dependencies flow **upward** — a crate may only depend on crates above it. ## Landmines & gotchas +- **`board add` accepts probe-rs `list` output directly.** `paavo-cli board add + --probe` takes a probe-rs selector token (`1fc9:0143-0:SERIAL`, where `-0` is + the USB interface) or a full pasted `probe-rs list` line. VID/PID are + validated as hex at registration (CLI **and** `POST /boards`), not deferred to + `probe_attach`. The canonical parser is `ProbeSelector::parse` in + `paavo-proto`. - **Workspace-excluded crates won't build with the host toolchain** (with one exception, noted below). These are intentionally outside `[workspace] members` (see `Cargo.toml` `exclude`): `tests/fixtures/smoke-crate`, `soak-tests/`, diff --git a/Cargo.lock b/Cargo.lock index d9f6bc4..0cf73ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2334,6 +2334,7 @@ version = "0.1.0" dependencies = [ "serde", "serde_json", + "thiserror 2.0.18", "ulid", ] diff --git a/contrib/99-probes.rules b/contrib/99-probes.rules index 90ac62f..729286a 100644 --- a/contrib/99-probes.rules +++ b/contrib/99-probes.rules @@ -2,10 +2,17 @@ # Source: https://probe.rs/docs/getting-started/probe-setup/ # Daplink (e.g. mcxa eval boards) -SUBSYSTEM=="usb", ATTRS{idVendor}=="0d28", ATTRS{idProduct}=="0204", MODE="0660", GROUP="plugdev" +SUBSYSTEM=="usb", ATTRS{idVendor}=="0d28", ATTRS{idProduct}=="0204", MODE="0660", GROUP="paavo" + +# NXP MCU-Link / on-board CMSIS-DAP (e.g. MCX-A266 EVK) +SUBSYSTEM=="usb", ATTRS{idVendor}=="1fc9", MODE="0660", GROUP="paavo" # Segger J-Link -SUBSYSTEM=="usb", ATTRS{idVendor}=="1366", MODE="0660", GROUP="plugdev" +SUBSYSTEM=="usb", ATTRS{idVendor}=="1366", MODE="0660", GROUP="paavo" # ST-Link (in case you mix) -SUBSYSTEM=="usb", ATTRS{idVendor}=="0483", ATTRS{idProduct}=="3748", MODE="0660", GROUP="plugdev" +SUBSYSTEM=="usb", ATTRS{idVendor}=="0483", ATTRS{idProduct}=="3748", MODE="0660", GROUP="paavo" + +# Single-purpose HIL box? Instead of maintaining a per-VID allowlist above, +# grant the paavo service every USB device with one rule: +# SUBSYSTEM=="usb", ENV{DEVTYPE}=="usb_device", MODE="0660", GROUP="paavo" diff --git a/contrib/README.md b/contrib/README.md index 8d9d0cd..74f687f 100644 --- a/contrib/README.md +++ b/contrib/README.md @@ -11,6 +11,14 @@ sudo install -m 0644 paavo.toml.example /etc/paavo/paavo.toml # then edit # still lives in the state_dir (/var/lib/paavo, owned via StateDirectory=). sudo useradd --system --create-home --home-dir /home/paavo --shell /bin/bash paavo sudo chown paavo:paavo /var/lib/paavo # StateDirectory= also enforces this on start +# paavod builds submitted test crates with cargo; ProtectHome=true in the unit +# hides /home, so install the toolchain UNDER the state dir (matches the +# CARGO_HOME/RUSTUP_HOME in paavod.service). Needs crates.io egress. +sudo -u paavo env CARGO_HOME=/var/lib/paavo/.cargo RUSTUP_HOME=/var/lib/paavo/.rustup \ + sh -c 'curl --proto "=https" --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --no-modify-path' +# add the embedded target(s) your boards build for (MCX-A266 shown): +sudo -u paavo env CARGO_HOME=/var/lib/paavo/.cargo RUSTUP_HOME=/var/lib/paavo/.rustup \ + /var/lib/paavo/.cargo/bin/rustup target add thumbv8m.main-none-eabihf sudo install -m 0755 ../target/release/paavod /usr/local/bin/ sudo install -m 0755 ../target/release/paavo-web /usr/local/bin/ sudo install -m 0644 paavod.service /etc/systemd/system/ diff --git a/contrib/paavod.service b/contrib/paavod.service index 4efcab5..a2bcc69 100644 --- a/contrib/paavod.service +++ b/contrib/paavod.service @@ -12,6 +12,12 @@ Restart=on-failure RestartSec=5 StateDirectory=paavo ReadWritePaths=/var/lib/paavo +# paavod builds submitted test crates with cargo. ProtectHome=true hides /home, +# so the toolchain must live under the (writable) state dir — see contrib/README.md +# for the one-time install. These must match RUSTUP_HOME/CARGO_HOME there. +Environment=CARGO_HOME=/var/lib/paavo/.cargo +Environment=RUSTUP_HOME=/var/lib/paavo/.rustup +Environment=PATH=/var/lib/paavo/.cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin ProtectSystem=strict ProtectHome=true NoNewPrivileges=true diff --git a/crates/paavo-cli/src/cli.rs b/crates/paavo-cli/src/cli.rs index 95ae9b0..c29ba08 100644 --- a/crates/paavo-cli/src/cli.rs +++ b/crates/paavo-cli/src/cli.rs @@ -142,7 +142,9 @@ pub enum BoardOp { /// Instance id (e.g. mcxa266-02). #[arg(long)] instance: String, - /// VID:PID:serial. + /// Probe selector. Accepts a probe-rs selector token + /// (`1fc9:0143:SERIAL`, optional `-IFACE`) or a full `probe-rs list` + /// line pasted verbatim. #[arg(long)] probe: String, /// probe-rs chip name. diff --git a/crates/paavo-cli/src/cmd_boards.rs b/crates/paavo-cli/src/cmd_boards.rs index 77bd169..ddcfcc3 100644 --- a/crates/paavo-cli/src/cmd_boards.rs +++ b/crates/paavo-cli/src/cmd_boards.rs @@ -32,23 +32,17 @@ pub async fn op(client: &Client, op: BoardOp) -> Result<()> { target, wiring_profile, } => { - let mut parts = probe.split(':'); - let vid = parts - .next() - .ok_or_else(|| anyhow!("probe missing VID"))? - .to_string(); - let pid = parts - .next() - .ok_or_else(|| anyhow!("probe missing PID"))? - .to_string(); - let serial = parts - .next() - .ok_or_else(|| anyhow!("probe missing serial"))? - .to_string(); + let probe_selector = ProbeSelector::parse(&probe).map_err(|e| { + anyhow!( + "invalid --probe {probe:?}: {e}\n\n\ + Paste a probe-rs selector (`1fc9:0143:SERIAL`) or a full \ + `probe-rs list` line." + ) + })?; let spec = BoardSpec { id: instance, kind, - probe_selector: ProbeSelector { vid, pid, serial }, + probe_selector, chip_name: chip, target_name: target, wiring_profile: Some(wiring_profile), diff --git a/crates/paavo-cli/src/cmd_run.rs b/crates/paavo-cli/src/cmd_run.rs index 65e3d63..ef05a07 100644 --- a/crates/paavo-cli/src/cmd_run.rs +++ b/crates/paavo-cli/src/cmd_run.rs @@ -387,6 +387,7 @@ mod tests { vid: "1366".into(), pid: "1015".into(), serial: format!("S-{id}"), + interface: None, }, chip_name: "MCXA266".into(), target_name: format!("target-{kind}"), diff --git a/crates/paavo-cli/tests/board_add_accepts_list_line.rs b/crates/paavo-cli/tests/board_add_accepts_list_line.rs new file mode 100644 index 0000000..88c4b75 --- /dev/null +++ b/crates/paavo-cli/tests/board_add_accepts_list_line.rs @@ -0,0 +1,121 @@ +//! End-to-end proof that `paavo-cli board add --probe ""` threads the *parsed* selector — including the `-IFACE` interface +//! suffix — all the way through `POST /boards` and into the DB. +//! +//! Harness mirrors `cli_run_board_resolution.rs`: build a `Config`, open a +//! `Db` in a tempdir, wrap it in the same `Arc>` the `AppState` +//! uses, build the router via `paavod::app::build_router`, bind a real +//! `TcpListener`, and `tokio::spawn` `axum::serve` so the daemon is reachable +//! over TCP (the `board add` command performs a real `POST /boards`). +//! +//! Because the served daemon writes through the *same* `Arc>` +//! handle we hold here, we read the stored row straight back with +//! `BoardRow::find` (no extra dependency, no second sqlite connection) and +//! assert the daemon actually persisted the normalized selector. + +use assert_cmd::Command as AssertCommand; +use paavo_db::{BoardRow, Db}; +use paavod::app::build_router; +use paavod::app_state::{AppState, DrainState}; +use paavod::cancellation::CancellationRegistry; +use paavod::config::{ + BuildCacheConfig, Config, QuarantineConfig, RetentionConfig, SchedulerConfig, ServerConfig, + TimeoutsConfig, WebConfig, +}; +use paavod::job_logs::JobLogsBroker; +use parking_lot::Mutex; +use std::sync::Arc; +use tempfile::tempdir; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn board_add_accepts_full_probe_rs_list_line() { + let tmp = tempdir().unwrap(); + let sd = paavod::state_dir::StateDir::from_root(tmp.path()); + sd.ensure_dirs().unwrap(); + + // Keep our own clone of the shared DB handle so we can read the row back + // after the CLI's POST commits. The served daemon mutates *this* handle. + let db = Arc::new(Mutex::new(Db::open(&sd.sqlite_path).unwrap())); + + let cfg = Arc::new(Config { + server: ServerConfig { + bind: "127.0.0.1:0".into(), + state_dir: tmp.path().to_path_buf(), + max_upload_bytes: 256 * 1024 * 1024, + }, + web: WebConfig { + bind: "127.0.0.1:0".into(), + }, + timeouts: TimeoutsConfig::default(), + scheduler: SchedulerConfig { + nightly_cron: "0 0 19 * * *".into(), + starvation_threshold_s: 21_600, + max_concurrent_builds: 5, + }, + build_cache: BuildCacheConfig::default(), + retention: RetentionConfig::default(), + quarantine: QuarantineConfig::default(), + corpus: vec![], + }); + let state = AppState { + db: db.clone(), + config: cfg, + inventory: Arc::new(Mutex::new(vec![])), + drain: DrainState::default(), + cancellation: CancellationRegistry::default(), + build_cancel: paavod::cancellation::BuildCancelRegistry::default(), + job_logs: JobLogsBroker::new(), + }; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let app = build_router(state); + let server = tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + // Paste a full `probe-rs list` line verbatim. The `-0` after the PID is + // the USB interface index; the CLI must parse it client-side and ship the + // structured selector to the daemon. + AssertCommand::cargo_bin("paavo-cli") + .unwrap() + .env("PAAVO_HOST", format!("http://{addr}")) + .args([ + "board", + "add", + "--kind", + "mcxa266", + "--instance", + "mcxa266-77", + "--probe", + "[0]: MCU-LINK on-board (r2E4) CMSIS-DAP V3.172 \ + -- 1fc9:0143-0:EDFHUAFM4J5ZJ (CMSIS-DAP)", + "--chip", + "MCXA266VFL", + "--target", + "frdm-mcx-a266", + ]) + .assert() + .success() + .stdout(predicates::str::contains("added: mcxa266-77")); + + // Read the row the daemon just persisted, through the same shared handle. + let stored = { + let guard = db.lock(); + BoardRow::find(guard.raw_conn(), "mcxa266-77") + .unwrap() + .expect("board mcxa266-77 should have been inserted by POST /boards") + }; + + let sel = &stored.spec.probe_selector; + assert_eq!(sel.vid, "1fc9"); + assert_eq!(sel.pid, "0143"); + assert_eq!(sel.serial, "EDFHUAFM4J5ZJ"); + assert_eq!( + sel.interface, + Some(0), + "the `-0` interface suffix must survive the CLI -> POST /boards -> DB round-trip" + ); + + server.abort(); +} diff --git a/crates/paavo-cli/tests/board_add_selector.rs b/crates/paavo-cli/tests/board_add_selector.rs new file mode 100644 index 0000000..ed8ea6a --- /dev/null +++ b/crates/paavo-cli/tests/board_add_selector.rs @@ -0,0 +1,28 @@ +//! `board add` must reject a malformed --probe locally, before any network +//! call. We point PAAVO_HOST at an unreachable address; if the command fails +//! with the parse message (not a connection error) we know it never POSTed. + +use assert_cmd::Command as AssertCommand; +use predicates::prelude::PredicateBooleanExt; + +#[test] +fn board_add_rejects_invalid_probe_before_network() { + let mut cmd = AssertCommand::cargo_bin("paavo-cli").unwrap(); + cmd.env("PAAVO_HOST", "http://127.0.0.1:1").args([ + "board", + "add", + "--kind", + "mcxa266", + "--instance", + "mcxa266-99", + "--probe", + "zz:gg:NOSER", + "--chip", + "MCXA266VFL", + "--target", + "frdm-mcx-a266", + ]); + cmd.assert().failure().stderr( + predicates::str::contains("invalid --probe").and(predicates::str::contains("bad VID")), + ); +} diff --git a/crates/paavo-cli/tests/cli_run_board_resolution.rs b/crates/paavo-cli/tests/cli_run_board_resolution.rs index 3570302..5a947ae 100644 --- a/crates/paavo-cli/tests/cli_run_board_resolution.rs +++ b/crates/paavo-cli/tests/cli_run_board_resolution.rs @@ -31,6 +31,7 @@ fn spec(id: &str, kind: &str) -> BoardSpec { vid: "1366".into(), pid: "1015".into(), serial: format!("S-{id}"), + interface: None, }, chip_name: "MCXA266".into(), target_name: format!("target-{kind}"), diff --git a/crates/paavo-core/tests/common/mod.rs b/crates/paavo-core/tests/common/mod.rs index be890b3..aa58175 100644 --- a/crates/paavo-core/tests/common/mod.rs +++ b/crates/paavo-core/tests/common/mod.rs @@ -45,6 +45,7 @@ pub fn insert_board(db: &Db, id: &str, kind: &str, health: BoardHealth) -> Board vid: "1366".into(), pid: "1015".into(), serial: id.into(), + interface: None, }, chip_name: "X".into(), target_name: format!("target-{kind}"), diff --git a/crates/paavo-db/src/job.rs b/crates/paavo-db/src/job.rs index 2692594..1900aed 100644 --- a/crates/paavo-db/src/job.rs +++ b/crates/paavo-db/src/job.rs @@ -928,6 +928,7 @@ mod tests { vid: "1366".into(), pid: "1015".into(), serial: id.into(), + interface: None, }, chip_name: "MCXA266VFL".into(), target_name: "frdm-mcx-a266".into(), diff --git a/crates/paavo-db/tests/board_ops.rs b/crates/paavo-db/tests/board_ops.rs index 6836c4a..133cfff 100644 --- a/crates/paavo-db/tests/board_ops.rs +++ b/crates/paavo-db/tests/board_ops.rs @@ -19,6 +19,7 @@ fn sample_board() -> BoardSpec { vid: "1366".into(), pid: "1015".into(), serial: "ABC".into(), + interface: None, }, chip_name: "MCXA266VFL".into(), target_name: "frdm-mcx-a266".into(), diff --git a/crates/paavo-db/tests/job_ops.rs b/crates/paavo-db/tests/job_ops.rs index d0ec2ce..e076a01 100644 --- a/crates/paavo-db/tests/job_ops.rs +++ b/crates/paavo-db/tests/job_ops.rs @@ -25,6 +25,7 @@ fn insert_default_board(db: &Db) { vid: "1366".into(), pid: "1015".into(), serial: "ABC".into(), + interface: None, }, chip_name: "MCXA266VFL".into(), target_name: "frdm-mcx-a266".into(), diff --git a/crates/paavo-probe/src/session.rs b/crates/paavo-probe/src/session.rs index a5714cb..31ead6b 100644 --- a/crates/paavo-probe/src/session.rs +++ b/crates/paavo-probe/src/session.rs @@ -161,7 +161,7 @@ impl RealSession { let selector = DebugProbeSelector { vendor_id: vid, product_id: pid, - interface: None, + interface: opts.probe_selector.interface, serial_number: serial_filter.clone(), }; let probe = lister.open(selector).map_err(|e| { diff --git a/crates/paavo-probe/tests/real_session_connect.rs b/crates/paavo-probe/tests/real_session_connect.rs index 2aae4d6..48415f1 100644 --- a/crates/paavo-probe/tests/real_session_connect.rs +++ b/crates/paavo-probe/tests/real_session_connect.rs @@ -72,6 +72,7 @@ fn connect_flashes_and_returns_live_session() { vid: "1fc9".into(), // NXP pid: "0143".into(), // MCU-Link CMSIS-DAP serial: "EDFHUAFM4J5ZJ".into(), // Felipe's specific EVK + interface: None, }, chip_name: "MCXA276".into(), // NOT MCXA266; spike finding elf_path: elf_fixture(), diff --git a/crates/paavo-probe/tests/real_session_drive.rs b/crates/paavo-probe/tests/real_session_drive.rs index 6dcf5a9..b6be089 100644 --- a/crates/paavo-probe/tests/real_session_drive.rs +++ b/crates/paavo-probe/tests/real_session_drive.rs @@ -66,6 +66,7 @@ fn next_event_streams_decoded_test_ok_then_bkpt() { vid: "1fc9".into(), // NXP pid: "0143".into(), // MCU-Link CMSIS-DAP serial: "EDFHUAFM4J5ZJ".into(), // Felipe's specific EVK + interface: None, }, chip_name: "MCXA276".into(), // NOT MCXA266; spike finding elf_path: elf_fixture(), diff --git a/crates/paavo-probe/tests/real_session_ram_boot.rs b/crates/paavo-probe/tests/real_session_ram_boot.rs index 43c7e3e..29662dc 100644 --- a/crates/paavo-probe/tests/real_session_ram_boot.rs +++ b/crates/paavo-probe/tests/real_session_ram_boot.rs @@ -187,6 +187,7 @@ fn connect_takes_ram_boot_path_with_vector_table_at_ram_origin() { vid: "1fc9".into(), // NXP pid: "0143".into(), // MCU-Link CMSIS-DAP serial: "EDFHUAFM4J5ZJ".into(), // Felipe's specific EVK + interface: None, }, chip_name: "MCXA276".into(), // NOT MCXA266; spike finding elf_path: elf_fixture(), diff --git a/crates/paavo-proto/Cargo.toml b/crates/paavo-proto/Cargo.toml index ea3c703..0091d40 100644 --- a/crates/paavo-proto/Cargo.toml +++ b/crates/paavo-proto/Cargo.toml @@ -9,6 +9,7 @@ description = "Wire types and protocol definitions shared across the paavo works [dependencies] serde = { workspace = true } +thiserror = { workspace = true } ulid = { workspace = true } [dev-dependencies] diff --git a/crates/paavo-proto/src/board.rs b/crates/paavo-proto/src/board.rs index a1bcddd..12eeeae 100644 --- a/crates/paavo-proto/src/board.rs +++ b/crates/paavo-proto/src/board.rs @@ -1,16 +1,174 @@ //! Board inventory and selector types. use serde::{Deserialize, Serialize}; +use thiserror::Error; -/// VID/PID/serial selector for a probe, matching the probe-rs naming. +/// VID/PID/serial/interface selector for a probe, matching the probe-rs naming. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ProbeSelector { - /// USB vendor id, hex string e.g. `"1366"`. + /// USB vendor id, hex string e.g. `"1fc9"`. Canonical form is lowercase 4-hex. pub vid: String, - /// USB product id, hex string e.g. `"1015"`. + /// USB product id, hex string e.g. `"0143"`. Canonical form is lowercase 4-hex. pub pid: String, - /// Probe serial number as reported by USB. + /// Probe serial number as reported by USB. May be empty ("no filter") + /// and may contain `:` (e.g. ESP JTAG MAC serials). pub serial: String, + /// USB interface index (the `-N` in a probe-rs selector). `None` matches + /// any interface; set only for multi-interface probes (e.g. FTDI). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub interface: Option, +} + +/// Error parsing a probe-rs selector string into a [`ProbeSelector`]. +#[derive(Debug, Error)] +pub enum ProbeSelectorParseError { + /// Selector was empty or had no PID part. + #[error( + "selector is empty or missing the PID (expected `VID:PID[-IFACE][:SERIAL]`, \ + e.g. `1fc9:0143:ABCD1234`)" + )] + Format, + /// VID was not a hex u16. + #[error("bad VID {value:?}: {source}")] + BadVid { + /// The offending VID text. + value: String, + /// The underlying parse error. + source: std::num::ParseIntError, + }, + /// PID was not a hex u16. + #[error("bad PID {value:?}: {source}")] + BadPid { + /// The offending PID text. + value: String, + /// The underlying parse error. + source: std::num::ParseIntError, + }, + /// Interface suffix was not a u8. + #[error("bad USB interface {value:?}: {source}")] + BadInterface { + /// The offending interface text. + value: String, + /// The underlying parse error. + source: std::num::ParseIntError, + }, +} + +impl ProbeSelector { + /// Parse a probe-rs selector token **or** a full `probe-rs list` line. + /// + /// Grammar matches probe-rs's own `DebugProbeSelector`: + /// `VID:PID[-IFACE][:SERIAL]`, split via `splitn(3, ':')` so colon-bearing + /// serials (e.g. ESP JTAG MACs) survive. VID/PID are hex and are + /// normalized to lowercase 4-hex. + pub fn parse(input: &str) -> Result { + let token = extract_selector_token(input); + + let mut parts = token.splitn(3, ':'); + let vid_raw = parts.next().unwrap_or("").trim(); + let pid_field = parts.next().ok_or(ProbeSelectorParseError::Format)?.trim(); + let serial = parts.next().map(|s| s.to_string()).unwrap_or_default(); + + // Peel the optional `-IFACE` off the PID field. + let (pid_raw, interface) = match pid_field.split_once('-') { + Some((pid, iface)) => { + let iface = iface.trim(); + let interface = if iface.is_empty() { + None + } else { + Some(iface.parse::().map_err(|source| { + ProbeSelectorParseError::BadInterface { + value: iface.to_string(), + source, + } + })?) + }; + (pid.trim(), interface) + } + None => (pid_field, None), + }; + + if vid_raw.is_empty() || pid_raw.is_empty() { + return Err(ProbeSelectorParseError::Format); + } + + let vid = parse_hex_u16(vid_raw).map_err(|source| ProbeSelectorParseError::BadVid { + value: vid_raw.to_string(), + source, + })?; + let pid = parse_hex_u16(pid_raw).map_err(|source| ProbeSelectorParseError::BadPid { + value: pid_raw.to_string(), + source, + })?; + + Ok(ProbeSelector { + vid: format!("{vid:04x}"), + pid: format!("{pid:04x}"), + serial, + interface, + }) + } + + /// Validate that `vid`/`pid` are hex `u16`. Used by paavod at registration + /// (the wire already carries a structured selector, so there's nothing to + /// re-split — just confirm the fields are well-formed). + pub fn validate(&self) -> Result<(), ProbeSelectorParseError> { + parse_hex_u16(&self.vid).map_err(|source| ProbeSelectorParseError::BadVid { + value: self.vid.clone(), + source, + })?; + parse_hex_u16(&self.pid).map_err(|source| ProbeSelectorParseError::BadPid { + value: self.pid.clone(), + source, + })?; + Ok(()) + } + + /// Validate **and** normalize `vid`/`pid` in place to canonical lowercase + /// 4-hex (e.g. `"1FC9"`/`"143"` → `"1fc9"`/`"0143"`). `serial` and + /// `interface` are left unchanged. Returns the same `BadVid`/`BadPid` + /// errors as [`ProbeSelector::validate`] when the fields are not hex. + pub fn canonicalize(&mut self) -> Result<(), ProbeSelectorParseError> { + let vid = parse_hex_u16(&self.vid).map_err(|source| ProbeSelectorParseError::BadVid { + value: self.vid.clone(), + source, + })?; + let pid = parse_hex_u16(&self.pid).map_err(|source| ProbeSelectorParseError::BadPid { + value: self.pid.clone(), + source, + })?; + self.vid = format!("{vid:04x}"); + self.pid = format!("{pid:04x}"); + Ok(()) + } +} + +/// Extract the `VID:PID…` token from either a bare token or a full +/// `probe-rs list` line (`[N]: -- ()`). +fn extract_selector_token(input: &str) -> &str { + let s = input.trim(); + match s.rfind(" -- ") { + Some(idx) => { + let after = s[idx + 4..].trim(); + // Strip a trailing ` (TYPE)` parenthetical only on the list-line path. + match after.rfind(" (") { + Some(p) => after[..p].trim(), + None => after, + } + } + None => s, + } +} + +/// Parse a hex string into `u16`, tolerating a `0x`/`0X` prefix and whitespace. +/// Kept in sync with `paavo_probe::session::parse_hex_u16` (see spec future-work). +fn parse_hex_u16(s: &str) -> Result { + let s = s.trim(); + let stripped = s + .strip_prefix("0x") + .or_else(|| s.strip_prefix("0X")) + .unwrap_or(s); + u16::from_str_radix(stripped, 16) } /// Whether a board is currently eligible to receive jobs. @@ -103,3 +261,171 @@ pub struct BoardView { /// Epoch ms when this board was first registered. pub created_at: i64, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_bare_token() { + let s = ProbeSelector::parse("1fc9:0143:EDFHUAFM4J5ZJ").unwrap(); + assert_eq!( + s, + ProbeSelector { + vid: "1fc9".into(), + pid: "0143".into(), + serial: "EDFHUAFM4J5ZJ".into(), + interface: None, + } + ); + } + + #[test] + fn parse_with_interface() { + let s = ProbeSelector::parse("1fc9:0143-0:EDFHUAFM4J5ZJ").unwrap(); + assert_eq!(s.pid, "0143"); + assert_eq!(s.interface, Some(0)); + } + + #[test] + fn parse_empty_interface_is_none() { + let s = ProbeSelector::parse("1fc9:0143-:S").unwrap(); + assert_eq!(s.interface, None); + assert_eq!(s.serial, "S"); + } + + #[test] + fn parse_colon_serial_preserved() { + let s = ProbeSelector::parse("303a:1001:DC:DA:0C:D3:FE:D8").unwrap(); + assert_eq!(s.vid, "303a"); + assert_eq!(s.pid, "1001"); + assert_eq!(s.serial, "DC:DA:0C:D3:FE:D8"); + } + + #[test] + fn parse_no_serial_is_empty() { + let s = ProbeSelector::parse("1fc9:0143").unwrap(); + assert_eq!(s.serial, ""); + assert_eq!(s.interface, None); + } + + #[test] + fn parse_full_probe_rs_list_line() { + let line = "[0]: MCU-LINK on-board (r2E4) CMSIS-DAP V3.172 \ + -- 1fc9:0143-0:EDFHUAFM4J5ZJ (CMSIS-DAP)"; + let s = ProbeSelector::parse(line).unwrap(); + assert_eq!( + s, + ProbeSelector { + vid: "1fc9".into(), + pid: "0143".into(), + serial: "EDFHUAFM4J5ZJ".into(), + interface: Some(0), + } + ); + } + + #[test] + fn parse_empty_input_is_format_error() { + assert!(matches!( + ProbeSelector::parse(""), + Err(ProbeSelectorParseError::Format) + )); + } + + #[test] + fn parse_normalizes_hex() { + let s = ProbeSelector::parse("0X1FC9:143:S").unwrap(); + assert_eq!(s.vid, "1fc9"); + assert_eq!(s.pid, "0143"); + + // Whitespace around the inner fields is tolerated and normalized too. + let s = ProbeSelector::parse(" 1fc9 : 0143 :S").unwrap(); + assert_eq!(s.vid, "1fc9"); + assert_eq!(s.pid, "0143"); + } + + #[test] + fn parse_rejects_bad_vid() { + assert!(matches!( + ProbeSelector::parse("zz:0143:S"), + Err(ProbeSelectorParseError::BadVid { .. }) + )); + } + + #[test] + fn parse_rejects_bad_pid() { + assert!(matches!( + ProbeSelector::parse("1fc9:gg:S"), + Err(ProbeSelectorParseError::BadPid { .. }) + )); + } + + #[test] + fn parse_rejects_missing_pid() { + assert!(matches!( + ProbeSelector::parse("1fc9"), + Err(ProbeSelectorParseError::Format) + )); + } + + #[test] + fn parse_rejects_bad_interface() { + assert!(matches!( + ProbeSelector::parse("1fc9:0143-x:S"), + Err(ProbeSelectorParseError::BadInterface { .. }) + )); + } + + #[test] + fn validate_accepts_normalized() { + let s = ProbeSelector { + vid: "1fc9".into(), + pid: "0143".into(), + serial: "S".into(), + interface: None, + }; + assert!(s.validate().is_ok()); + } + + #[test] + fn validate_rejects_bad_hex() { + let s = ProbeSelector { + vid: "zz".into(), + pid: "0143".into(), + serial: "S".into(), + interface: None, + }; + assert!(s.validate().is_err()); + } + + #[test] + fn canonicalize_normalizes_vid_pid_in_place() { + let mut s = ProbeSelector { + vid: "1FC9".into(), + pid: "143".into(), + serial: "S".into(), + interface: Some(0), + }; + s.canonicalize().unwrap(); + assert_eq!(s.vid, "1fc9"); + assert_eq!(s.pid, "0143"); + // serial + interface are untouched. + assert_eq!(s.serial, "S"); + assert_eq!(s.interface, Some(0)); + } + + #[test] + fn canonicalize_rejects_bad_hex() { + let mut s = ProbeSelector { + vid: "zz".into(), + pid: "0143".into(), + serial: "S".into(), + interface: None, + }; + assert!(matches!( + s.canonicalize(), + Err(ProbeSelectorParseError::BadVid { .. }) + )); + } +} diff --git a/crates/paavo-proto/src/stats.rs b/crates/paavo-proto/src/stats.rs index cc14a16..91f56cb 100644 --- a/crates/paavo-proto/src/stats.rs +++ b/crates/paavo-proto/src/stats.rs @@ -215,6 +215,7 @@ mod tests { vid: "1366".into(), pid: "1015".into(), serial: "ABC".into(), + interface: None, }, chip_name: "MCXA266VFL".into(), target_name: "frdm-mcx-a266".into(), diff --git a/crates/paavo-proto/tests/board_view.rs b/crates/paavo-proto/tests/board_view.rs index 12e5ceb..2b535e3 100644 --- a/crates/paavo-proto/tests/board_view.rs +++ b/crates/paavo-proto/tests/board_view.rs @@ -10,6 +10,7 @@ fn board_view_round_trips_through_json() { vid: "1366".into(), pid: "1015".into(), serial: "ABC".into(), + interface: None, }, chip_name: "X".into(), target_name: "T".into(), @@ -43,6 +44,7 @@ fn board_view_omits_none_quarantine_reason() { vid: "1".into(), pid: "2".into(), serial: "S".into(), + interface: None, }, chip_name: "X".into(), target_name: "T".into(), diff --git a/crates/paavo-proto/tests/serde_roundtrip.rs b/crates/paavo-proto/tests/serde_roundtrip.rs index 1c82121..576b0d5 100644 --- a/crates/paavo-proto/tests/serde_roundtrip.rs +++ b/crates/paavo-proto/tests/serde_roundtrip.rs @@ -170,6 +170,7 @@ fn board_spec_roundtrip() { vid: "1366".into(), pid: "1015".into(), serial: "000123456789".into(), + interface: None, }, chip_name: "MCXA266VFL".into(), target_name: "frdm-mcx-a266".into(), @@ -202,6 +203,7 @@ fn board_selector_matches_kind_instance_and_wiring_profile() { vid: "1366".into(), pid: "1015".into(), serial: "abc".into(), + interface: None, }, chip_name: "MCXA266VFL".into(), target_name: "frdm-mcx-a266".into(), @@ -432,3 +434,35 @@ fn awaiting_board_serializes_to_snake_case() { assert_eq!(back, paavo_proto::JobState::AwaitingBoard); assert!(!back.is_terminal()); } + +#[test] +fn probe_selector_omits_interface_when_none() { + let sel = paavo_proto::ProbeSelector { + vid: "1fc9".into(), + pid: "0143".into(), + serial: "ABCD".into(), + interface: None, + }; + let j = serde_json::to_string(&sel).unwrap(); + assert!( + !j.contains("interface"), + "None interface must be omitted: {j}" + ); + assert_eq!(sel, serde_json::from_str(&j).unwrap()); +} + +#[test] +fn probe_selector_serializes_interface_when_some() { + let sel = paavo_proto::ProbeSelector { + vid: "0403".into(), + pid: "6010".into(), + serial: "X".into(), + interface: Some(1), + }; + let j = serde_json::to_string(&sel).unwrap(); + assert!( + j.contains("\"interface\":1"), + "Some interface must serialize: {j}" + ); + assert_eq!(sel, serde_json::from_str(&j).unwrap()); +} diff --git a/crates/paavo-web/tests/api_dashboard.rs b/crates/paavo-web/tests/api_dashboard.rs index bda9b6a..d4ce797 100644 --- a/crates/paavo-web/tests/api_dashboard.rs +++ b/crates/paavo-web/tests/api_dashboard.rs @@ -43,6 +43,7 @@ fn board(id: &str) -> BoardSpec { vid: "1366".into(), pid: "1015".into(), serial: "ABC".into(), + interface: None, }, chip_name: "MCXA266VFL".into(), target_name: "frdm-mcx-a266".into(), diff --git a/crates/paavod/src/main.rs b/crates/paavod/src/main.rs index 280fd17..a3f6a03 100644 --- a/crates/paavod/src/main.rs +++ b/crates/paavod/src/main.rs @@ -157,10 +157,19 @@ fn load_inventory( let parsed: Boards = toml::from_str(&raw).with_context(|| format!("parsing {}", boards_toml.display()))?; for spec in &parsed.board { + // Canonicalize (normalize + validate) each seed so stored selectors + // are uniform lowercase 4-hex — matching what POST /boards does — and + // a malformed selector fails fast here rather than blowing up later at + // probe_attach. boards.toml is operator input, so a clear startup + // error beats a silent bad insert. + let mut spec = spec.clone(); + spec.probe_selector + .canonicalize() + .with_context(|| format!("invalid probe_selector for board {}", spec.id))?; if paavo_db::BoardRow::find(db.raw_conn(), &spec.id)?.is_none() { paavo_db::BoardRow::insert( db.raw_conn(), - spec, + &spec, chrono::Utc::now().timestamp_millis(), ) .with_context(|| format!("inserting board {}", spec.id))?; @@ -186,3 +195,90 @@ impl paavo_core::Runner for FakeRunner { } } } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + + fn write_boards_toml(dir: &std::path::Path, body: &str) -> std::path::PathBuf { + let p = dir.join("boards.toml"); + let mut f = std::fs::File::create(&p).unwrap(); + f.write_all(body.as_bytes()).unwrap(); + p + } + + #[test] + fn load_inventory_rejects_bad_selector_in_boards_toml() { + let tmp = tempfile::tempdir().unwrap(); + let db = paavo_db::Db::open(tmp.path().join("paavo.sqlite")).unwrap(); + // The selector vid is non-hex; every other field is well-formed so the + // TOML deserializes cleanly and the ONLY failure is selector validation. + let toml = write_boards_toml( + tmp.path(), + r#" +[[board]] +id = "bad-01" +kind = "mcxa266" +chip_name = "MCXA266VFL" +target_name = "frdm-mcx-a266" +wiring_profile = "default" +health = "healthy" +probe_selector = { vid = "zz", pid = "0143", serial = "S" } +"#, + ); + let err = load_inventory(&db, &toml).unwrap_err(); + let msg = format!("{err:#}"); + assert!( + msg.contains("invalid probe_selector"), + "error should be the selector validation, got: {msg}" + ); + } + + #[test] + fn load_inventory_accepts_good_selector() { + let tmp = tempfile::tempdir().unwrap(); + let db = paavo_db::Db::open(tmp.path().join("paavo.sqlite")).unwrap(); + let toml = write_boards_toml( + tmp.path(), + r#" +[[board]] +id = "ok-01" +kind = "mcxa266" +chip_name = "MCXA266VFL" +target_name = "frdm-mcx-a266" +wiring_profile = "default" +health = "healthy" +probe_selector = { vid = "1fc9", pid = "0143", serial = "S" } +"#, + ); + let inv = load_inventory(&db, &toml).unwrap(); + assert!(inv.iter().any(|b| b.id == "ok-01")); + } + + #[test] + fn load_inventory_canonicalizes_seeded_selector() { + let tmp = tempfile::tempdir().unwrap(); + let db = paavo_db::Db::open(tmp.path().join("paavo.sqlite")).unwrap(); + let toml = write_boards_toml( + tmp.path(), + r#" +[[board]] +id = "up-01" +kind = "mcxa266" +chip_name = "MCXA266VFL" +target_name = "frdm-mcx-a266" +wiring_profile = "default" +health = "healthy" +probe_selector = { vid = "1FC9", pid = "143", serial = "S" } +"#, + ); + let inv = load_inventory(&db, &toml).unwrap(); + let b = inv + .iter() + .find(|b| b.id == "up-01") + .expect("seeded board present"); + assert_eq!(b.probe_selector.vid, "1fc9"); + assert_eq!(b.probe_selector.pid, "0143"); + } +} diff --git a/crates/paavod/src/routes/boards.rs b/crates/paavod/src/routes/boards.rs index 9f06c9e..2adf7eb 100644 --- a/crates/paavod/src/routes/boards.rs +++ b/crates/paavod/src/routes/boards.rs @@ -43,7 +43,7 @@ pub async fn list_boards(State(s): State) -> HandlerResult, - Json(spec): Json, + Json(mut spec): Json, ) -> HandlerResult { if spec.health != BoardHealth::Healthy { return Err(( @@ -53,6 +53,15 @@ pub async fn add_board( .into(), )); } + // The wire carries vid/pid as free-form strings; normalize to canonical + // lowercase 4-hex (and reject non-hex) so stored selectors are uniform and + // never fail later at probe_attach. + spec.probe_selector.canonicalize().map_err(|e| { + ( + StatusCode::BAD_REQUEST, + format!("invalid probe_selector: {e}"), + ) + })?; let now_ms = Utc::now().timestamp_millis(); { let db = s.db.lock(); diff --git a/crates/paavod/tests/api_admin.rs b/crates/paavod/tests/api_admin.rs index dfca506..cf16632 100644 --- a/crates/paavod/tests/api_admin.rs +++ b/crates/paavod/tests/api_admin.rs @@ -68,6 +68,7 @@ fn sample_board() -> BoardSpec { vid: "1366".into(), pid: "1015".into(), serial: "ABC".into(), + interface: None, }, chip_name: "MCXA266VFL".into(), target_name: "frdm-mcx-a266".into(), diff --git a/crates/paavod/tests/api_boards.rs b/crates/paavod/tests/api_boards.rs index 8c3bf41..d3bf442 100644 --- a/crates/paavod/tests/api_boards.rs +++ b/crates/paavod/tests/api_boards.rs @@ -235,6 +235,7 @@ async fn quarantine_and_unquarantine_flip_health_and_cache() { vid: "x".into(), pid: "x".into(), serial: "x".into(), + interface: None, }, chip_name: "x".into(), target_name: "x".into(), @@ -370,6 +371,7 @@ async fn delete_board_with_referencing_job_returns_409() { vid: "x".into(), pid: "x".into(), serial: "x".into(), + interface: None, }, chip_name: "x".into(), target_name: "x".into(), @@ -415,3 +417,42 @@ async fn delete_board_with_referencing_job_returns_409() { "body should explain the FK conflict, got: {body}" ); } + +#[tokio::test] +async fn add_board_rejects_non_hex_vid() { + let app = build_router(state()); + let mut body = sample_board_json(); + body["probe_selector"]["vid"] = serde_json::json!("zz"); + let resp = post_json(app, "/boards", body).await; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = read_text(resp).await; + assert!( + body.contains("probe_selector"), + "body should name the failing field, got: {body}" + ); +} + +#[tokio::test] +async fn add_board_normalizes_vid_pid() { + let app = build_router(state()); + let mut body = sample_board_json(); + body["probe_selector"]["vid"] = serde_json::json!("1FC9"); // uppercase + body["probe_selector"]["pid"] = serde_json::json!("143"); // unpadded + let resp = post_json(app.clone(), "/boards", body).await; + assert_eq!(resp.status(), StatusCode::CREATED); + + // GET /boards and confirm the stored selector was canonicalized. + // Mirrors `post_boards_then_get_boards_returns_full_view`: build an + // inline GET request and decode the `Vec` body via + // `read_json`. The view flattens `BoardSpec`, so `probe_selector` is + // nested and reflects exactly what the daemon persisted. + let req = Request::builder() + .uri("/boards") + .body(axum::body::Body::empty()) + .unwrap(); + let v = read_json(app.oneshot(req).await.unwrap()).await; + let arr = v.as_array().unwrap(); + assert_eq!(arr.len(), 1); + assert_eq!(arr[0]["probe_selector"]["vid"], "1fc9"); + assert_eq!(arr[0]["probe_selector"]["pid"], "0143"); +} diff --git a/crates/paavod/tests/api_jobs.rs b/crates/paavod/tests/api_jobs.rs index f740be3..a6ab1df 100644 --- a/crates/paavod/tests/api_jobs.rs +++ b/crates/paavod/tests/api_jobs.rs @@ -25,6 +25,7 @@ fn state_with_upload_cap(tmp_root: &std::path::Path, max_upload_bytes: usize) -> vid: "x".into(), pid: "x".into(), serial: "x".into(), + interface: None, }, chip_name: "x".into(), target_name: "x".into(), diff --git a/crates/paavod/tests/cron_enqueue.rs b/crates/paavod/tests/cron_enqueue.rs index 52001d0..62ce899 100644 --- a/crates/paavod/tests/cron_enqueue.rs +++ b/crates/paavod/tests/cron_enqueue.rs @@ -45,6 +45,7 @@ fn make_state(corpus: Vec, state_root: &std::path::Path) -> AppStat vid: "x".into(), pid: "x".into(), serial: "x".into(), + interface: None, }, chip_name: "x".into(), target_name: "x".into(), diff --git a/crates/paavod/tests/dispatch_loop.rs b/crates/paavod/tests/dispatch_loop.rs index 8c0f4dd..9a845df 100644 --- a/crates/paavod/tests/dispatch_loop.rs +++ b/crates/paavod/tests/dispatch_loop.rs @@ -61,6 +61,7 @@ fn fixture_state(_out: JobOutcome) -> (AppState, paavo_proto::JobId, tempfile::T vid: "x".into(), pid: "x".into(), serial: "x".into(), + interface: None, }, chip_name: "x".into(), target_name: "x".into(), diff --git a/crates/paavod/tests/real_runner_end_to_end.rs b/crates/paavod/tests/real_runner_end_to_end.rs index 925085a..d72f0bb 100644 --- a/crates/paavod/tests/real_runner_end_to_end.rs +++ b/crates/paavod/tests/real_runner_end_to_end.rs @@ -125,6 +125,7 @@ fn real_runner_passes_against_real_mcxa266() { vid: "1fc9".into(), // NXP pid: "0143".into(), // MCU-Link CMSIS-DAP serial: "EDFHUAFM4J5ZJ".into(), // Felipe's specific EVK + interface: None, }, chip_name: "MCXA276".into(), target_name: "frdm-mcx-a266".into(), diff --git a/crates/paavod/tests/shutdown_flow.rs b/crates/paavod/tests/shutdown_flow.rs index 2c19477..2c50273 100644 --- a/crates/paavod/tests/shutdown_flow.rs +++ b/crates/paavod/tests/shutdown_flow.rs @@ -26,6 +26,7 @@ fn make_state(tmp: &std::path::Path) -> AppState { vid: "x".into(), pid: "x".into(), serial: "x".into(), + interface: None, }, chip_name: "x".into(), target_name: "x".into(), diff --git a/dev/seed-demo/src/main.rs b/dev/seed-demo/src/main.rs index f59e964..2aaa581 100644 --- a/dev/seed-demo/src/main.rs +++ b/dev/seed-demo/src/main.rs @@ -376,6 +376,7 @@ fn main() -> Result<()> { vid: "1366".into(), pid: "1015".into(), serial: format!("FAKE{i:02}"), + interface: None, }, chip_name: "MCXA266".into(), target_name: "thumbv8m.main-none-eabihf".into(), diff --git a/docs/deployment.md b/docs/deployment.md index 303fdff..77edcc1 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -35,7 +35,9 @@ Then follow [`contrib/README.md`](../contrib/README.md) for systemd + udev. - `sandboxes/` — per-job build dirs. - `cargo-target/` — shared `CARGO_TARGET_DIR` for cargo's incremental reuse. - `cache/elf/` — cached ELFs paired with `build_cache` table rows (DB row holds the path; the ELF file lives here). LRU evicted when `build_cache.max_bytes` is hit. -- `boards.toml` — `paavo-cli board add` writes this; restart paavod to pick up changes. +- `boards.toml` — declarative seed read at startup (inserts boards whose id is + not already in the DB). `paavo-cli board add` writes the **DB** via `POST + /boards`, not this file; edit `boards.toml` by hand to seed a fresh deploy. ## Updating diff --git a/docs/superpowers/plans/2026-06-29-probe-selector-parsing.md b/docs/superpowers/plans/2026-06-29-probe-selector-parsing.md new file mode 100644 index 0000000..98144ef --- /dev/null +++ b/docs/superpowers/plans/2026-06-29-probe-selector-parsing.md @@ -0,0 +1,734 @@ +# probe-rs-native selector parsing for `board add` — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let `paavo-cli board add --probe` accept probe-rs selector strings (bare token *or* a full `probe-rs list` line) directly, parse/validate them at registration, and preserve the USB interface index. + +**Architecture:** A canonical parser lives in `paavo-proto` on `ProbeSelector` (the type's owner). `paavo-cli` calls it to normalize before POST; `paavod` re-validates at `POST /boards`. `ProbeSelector` gains an optional `interface: Option` field (additive, no `deny_unknown_fields`, no DB migration) that `paavo-probe` threads into probe-rs's `DebugProbeSelector` at attach. + +**Tech Stack:** Rust 1.95, serde/serde_json, thiserror, axum (paavod), assert_cmd + predicates (CLI tests). Spec: `docs/superpowers/specs/2026-06-29-probe-selector-parsing-design.md`. + +--- + +## File structure + +- `crates/paavo-proto/src/board.rs` — add `interface` field; `ProbeSelectorParseError`; `ProbeSelector::parse` + `validate`; `parse_hex_u16` helper + `extract_selector_token` helper; `#[cfg(test)] mod tests`. +- `crates/paavo-proto/tests/serde_roundtrip.rs` — wire-compat tests for the new field. +- `crates/paavo-cli/src/cmd_boards.rs` — replace the naive `split(':')` with `ProbeSelector::parse`. +- `crates/paavo-cli/src/cli.rs` — reword `--probe` help. +- `crates/paavo-cli/tests/board_add_selector.rs` — new CLI fail-fast test. +- `crates/paavod/src/routes/boards.rs` — `validate()` → `400` in `add_board`. +- `crates/paavod/tests/api_boards.rs` — daemon rejects bad selector. +- `crates/paavo-probe/src/session.rs` — pass `interface` into `DebugProbeSelector`. +- Many test/src files — add `interface: None,` to existing `ProbeSelector { … }` literals (Task 1, compiler-driven). +- `AGENTS.md`, `docs/deployment.md` — doc fixes (Task 7). + +--- + +### Task 1: Add the `interface` field and repair every `ProbeSelector` literal + +Adding a field breaks all struct-literal construction sites. The new field is optional on the wire (serde default + skip-when-none), so behavior is unchanged; this task is purely the type change + mechanical literal repair, gated by a wire-compat test. + +**Files:** +- Modify: `crates/paavo-proto/src/board.rs:6-14` +- Test: `crates/paavo-proto/tests/serde_roundtrip.rs` (append) +- Modify (add `interface: None,`): every in-workspace `ProbeSelector { … }` literal (list below) + +- [ ] **Step 1: Write the failing wire-compat tests** + +Append to `crates/paavo-proto/tests/serde_roundtrip.rs`: + +```rust +#[test] +fn probe_selector_omits_interface_when_none() { + let sel = paavo_proto::ProbeSelector { + vid: "1fc9".into(), + pid: "0143".into(), + serial: "ABCD".into(), + interface: None, + }; + let j = serde_json::to_string(&sel).unwrap(); + assert!(!j.contains("interface"), "None interface must be omitted: {j}"); + assert_eq!(sel, serde_json::from_str(&j).unwrap()); +} + +#[test] +fn probe_selector_serializes_interface_when_some() { + let sel = paavo_proto::ProbeSelector { + vid: "0403".into(), + pid: "6010".into(), + serial: "X".into(), + interface: Some(1), + }; + let j = serde_json::to_string(&sel).unwrap(); + assert!(j.contains("\"interface\":1"), "Some interface must serialize: {j}"); + assert_eq!(sel, serde_json::from_str(&j).unwrap()); +} +``` + +- [ ] **Step 2: Run — expect a COMPILE failure (field doesn't exist yet)** + +Run: `cargo test -p paavo-proto --test serde_roundtrip` +Expected: FAIL to compile — `struct ProbeSelector has no field named interface`. + +- [ ] **Step 3: Add the field** + +In `crates/paavo-proto/src/board.rs`, replace the struct body (lines 6-14): + +```rust +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ProbeSelector { + /// USB vendor id, normalized lowercase 4-hex, e.g. `"1fc9"`. + pub vid: String, + /// USB product id, normalized lowercase 4-hex, e.g. `"0143"`. + pub pid: String, + /// Probe serial number as reported by USB. May be empty ("no filter") + /// and may contain `:` (e.g. ESP JTAG MAC serials). + pub serial: String, + /// USB interface index (the `-N` in a probe-rs selector). `None` matches + /// any interface; set only for multi-interface probes (e.g. FTDI). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub interface: Option, +} +``` + +- [ ] **Step 4: Add `interface: None,` to every existing literal (compiler-driven)** + +Run `cargo build --workspace --all-targets` and add `interface: None,` to each `ProbeSelector { … }` the compiler flags (E0063). The full in-workspace set: + +``` +crates/paavo-cli/tests/cli_run_board_resolution.rs:30 +crates/paavo-cli/src/cmd_run.rs:386 +crates/paavo-cli/src/cmd_boards.rs:51 (superseded in Task 4, but must compile now) +crates/paavo-core/tests/common/mod.rs:44 +crates/paavo-db/src/job.rs:927 +crates/paavo-db/tests/board_ops.rs:18 +crates/paavo-db/tests/job_ops.rs:24 +crates/paavo-proto/src/stats.rs:214 +crates/paavo-proto/tests/board_view.rs:9 +crates/paavo-proto/tests/board_view.rs:42 +crates/paavo-proto/tests/serde_roundtrip.rs:169 +crates/paavo-probe/tests/real_session_ram_boot.rs:186 +crates/paavo-probe/tests/real_session_connect.rs:71 +crates/paavo-probe/tests/real_session_drive.rs:65 +crates/paavod/tests/shutdown_flow.rs:25 +crates/paavod/tests/real_runner_end_to_end.rs:124 +crates/paavod/tests/api_jobs.rs:24 +crates/paavod/tests/api_boards.rs:234 +crates/paavod/tests/api_boards.rs:369 +crates/paavod/tests/dispatch_loop.rs:60 +crates/paavod/tests/api_admin.rs:67 +crates/paavod/tests/cron_enqueue.rs:44 +crates/paavo-web/tests/api_dashboard.rs:42 +``` + +Example edit (each site looks like this): + +```rust + probe_selector: ProbeSelector { + vid: "1366".into(), + pid: "1015".into(), + serial: "ABC".into(), + interface: None, // <-- add this line + }, +``` + +> Note: `dev/seed-demo/src/main.rs:375` also constructs `ProbeSelector` but is **workspace-excluded** (not built by `cargo build --workspace`); add `interface: None,` there too only if you build that dev tool, but it is out of CI scope. + +- [ ] **Step 5: Run — workspace compiles, wire-compat tests pass** + +Run: `cargo test --workspace` +Expected: PASS (the two new tests included; `interface` omitted when `None`, present when `Some`). + +- [ ] **Step 6: Commit** + +```bash +git add -A +git commit -m "feat(proto): add optional interface to ProbeSelector (additive wire field)" +``` + +--- + +### Task 2: Selector parser + validator in `paavo-proto` + +**Files:** +- Modify: `crates/paavo-proto/src/board.rs` (add error enum, `impl ProbeSelector`, two free helpers, tests module) + +- [ ] **Step 1: Write the failing unit tests** + +Append to `crates/paavo-proto/src/board.rs`: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_bare_token() { + let s = ProbeSelector::parse("1fc9:0143:EDFHUAFM4J5ZJ").unwrap(); + assert_eq!( + s, + ProbeSelector { + vid: "1fc9".into(), + pid: "0143".into(), + serial: "EDFHUAFM4J5ZJ".into(), + interface: None, + } + ); + } + + #[test] + fn parse_with_interface() { + let s = ProbeSelector::parse("1fc9:0143-0:EDFHUAFM4J5ZJ").unwrap(); + assert_eq!(s.pid, "0143"); + assert_eq!(s.interface, Some(0)); + } + + #[test] + fn parse_empty_interface_is_none() { + let s = ProbeSelector::parse("1fc9:0143-:S").unwrap(); + assert_eq!(s.interface, None); + assert_eq!(s.serial, "S"); + } + + #[test] + fn parse_colon_serial_preserved() { + let s = ProbeSelector::parse("303a:1001:DC:DA:0C:D3:FE:D8").unwrap(); + assert_eq!(s.vid, "303a"); + assert_eq!(s.pid, "1001"); + assert_eq!(s.serial, "DC:DA:0C:D3:FE:D8"); + } + + #[test] + fn parse_no_serial_is_empty() { + let s = ProbeSelector::parse("1fc9:0143").unwrap(); + assert_eq!(s.serial, ""); + assert_eq!(s.interface, None); + } + + #[test] + fn parse_normalizes_hex() { + let s = ProbeSelector::parse("0X1FC9:143:S").unwrap(); + assert_eq!(s.vid, "1fc9"); + assert_eq!(s.pid, "0143"); + } + + #[test] + fn parse_rejects_bad_vid() { + assert!(matches!( + ProbeSelector::parse("zz:0143:S"), + Err(ProbeSelectorParseError::BadVid { .. }) + )); + } + + #[test] + fn parse_rejects_bad_pid() { + assert!(matches!( + ProbeSelector::parse("1fc9:gg:S"), + Err(ProbeSelectorParseError::BadPid { .. }) + )); + } + + #[test] + fn parse_rejects_missing_pid() { + assert!(matches!( + ProbeSelector::parse("1fc9"), + Err(ProbeSelectorParseError::Format) + )); + } + + #[test] + fn parse_rejects_bad_interface() { + assert!(matches!( + ProbeSelector::parse("1fc9:0143-x:S"), + Err(ProbeSelectorParseError::BadInterface { .. }) + )); + } + + #[test] + fn validate_accepts_normalized() { + let s = ProbeSelector { + vid: "1fc9".into(), + pid: "0143".into(), + serial: "S".into(), + interface: None, + }; + assert!(s.validate().is_ok()); + } + + #[test] + fn validate_rejects_bad_hex() { + let s = ProbeSelector { + vid: "zz".into(), + pid: "0143".into(), + serial: "S".into(), + interface: None, + }; + assert!(s.validate().is_err()); + } +} +``` + +- [ ] **Step 2: Run — expect compile failure (`parse`/`validate`/error type missing)** + +Run: `cargo test -p paavo-proto board::tests` +Expected: FAIL to compile — no `parse`, `validate`, or `ProbeSelectorParseError`. + +- [ ] **Step 3: Implement the error type, parser, validator, and helpers** + +Add to `crates/paavo-proto/src/board.rs` (after the `ProbeSelector` struct). Note the existing import is `use serde::{Deserialize, Serialize};` — add `use thiserror::Error;` at the top of the file if not present (thiserror is a workspace dep used across the codebase). + +```rust +/// Error parsing a probe-rs selector string into a [`ProbeSelector`]. +#[derive(Debug, Error)] +pub enum ProbeSelectorParseError { + /// Selector was empty or had no PID part. + #[error("selector is empty or missing the PID (expected `VID:PID[-IFACE][:SERIAL]`, \ + e.g. `1fc9:0143:ABCD1234`)")] + Format, + /// VID was not a hex u16. + #[error("bad VID {value:?}: {source}")] + BadVid { + /// The offending VID text. + value: String, + /// The underlying parse error. + source: std::num::ParseIntError, + }, + /// PID was not a hex u16. + #[error("bad PID {value:?}: {source}")] + BadPid { + /// The offending PID text. + value: String, + /// The underlying parse error. + source: std::num::ParseIntError, + }, + /// Interface suffix was not a u8. + #[error("bad USB interface {value:?}: {source}")] + BadInterface { + /// The offending interface text. + value: String, + /// The underlying parse error. + source: std::num::ParseIntError, + }, +} + +impl ProbeSelector { + /// Parse a probe-rs selector token **or** a full `probe-rs list` line. + /// + /// Grammar matches probe-rs's own `DebugProbeSelector`: + /// `VID:PID[-IFACE][:SERIAL]`, split via `splitn(3, ':')` so colon-bearing + /// serials (e.g. ESP JTAG MACs) survive. VID/PID are hex and are + /// normalized to lowercase 4-hex. + pub fn parse(input: &str) -> Result { + let token = extract_selector_token(input); + + let mut parts = token.splitn(3, ':'); + let vid_raw = parts.next().unwrap_or("").trim(); + let pid_field = parts + .next() + .ok_or(ProbeSelectorParseError::Format)? + .trim(); + let serial = parts.next().map(|s| s.to_string()).unwrap_or_default(); + + // Peel the optional `-IFACE` off the PID field. + let (pid_raw, interface) = match pid_field.split_once('-') { + Some((pid, iface)) => { + let iface = iface.trim(); + let interface = if iface.is_empty() { + None + } else { + Some(iface.parse::().map_err(|source| { + ProbeSelectorParseError::BadInterface { + value: iface.to_string(), + source, + } + })?) + }; + (pid.trim(), interface) + } + None => (pid_field, None), + }; + + if vid_raw.is_empty() || pid_raw.is_empty() { + return Err(ProbeSelectorParseError::Format); + } + + let vid = parse_hex_u16(vid_raw).map_err(|source| ProbeSelectorParseError::BadVid { + value: vid_raw.to_string(), + source, + })?; + let pid = parse_hex_u16(pid_raw).map_err(|source| ProbeSelectorParseError::BadPid { + value: pid_raw.to_string(), + source, + })?; + + Ok(ProbeSelector { + vid: format!("{vid:04x}"), + pid: format!("{pid:04x}"), + serial, + interface, + }) + } + + /// Validate that `vid`/`pid` are hex `u16`. Used by paavod at registration + /// (the wire already carries a structured selector, so there's nothing to + /// re-split — just confirm the fields are well-formed). + pub fn validate(&self) -> Result<(), ProbeSelectorParseError> { + parse_hex_u16(&self.vid).map_err(|source| ProbeSelectorParseError::BadVid { + value: self.vid.clone(), + source, + })?; + parse_hex_u16(&self.pid).map_err(|source| ProbeSelectorParseError::BadPid { + value: self.pid.clone(), + source, + })?; + Ok(()) + } +} + +/// Extract the `VID:PID…` token from either a bare token or a full +/// `probe-rs list` line (`[N]: -- ()`). +fn extract_selector_token(input: &str) -> &str { + let s = input.trim(); + match s.rfind(" -- ") { + Some(idx) => { + let after = s[idx + 4..].trim(); + // Strip a trailing ` (TYPE)` parenthetical only on the list-line path. + match after.rfind(" (") { + Some(p) => after[..p].trim(), + None => after, + } + } + None => s, + } +} + +/// Parse a hex string into `u16`, tolerating a `0x`/`0X` prefix and whitespace. +/// Kept in sync with `paavo_probe::session::parse_hex_u16` (see spec future-work). +fn parse_hex_u16(s: &str) -> Result { + let s = s.trim(); + let stripped = s + .strip_prefix("0x") + .or_else(|| s.strip_prefix("0X")) + .unwrap_or(s); + u16::from_str_radix(stripped, 16) +} +``` + +- [ ] **Step 4: Run — token tests pass** + +Run: `cargo test -p paavo-proto board::tests` +Expected: PASS (all 12 tests). The full-`probe-rs list`-line case is added in Task 3. + +- [ ] **Step 5: Commit** + +```bash +git add crates/paavo-proto/src/board.rs +git commit -m "feat(proto): ProbeSelector::parse + validate (probe-rs token grammar)" +``` + +--- + +### Task 3: Accept a full `probe-rs list` line + +`extract_selector_token` already handles ` -- ` and the trailing parenthetical; this task proves it end-to-end and locks it with a test. + +**Files:** +- Modify: `crates/paavo-proto/src/board.rs` (`mod tests`) + +- [ ] **Step 1: Write the failing test** + +Add inside `mod tests` in `crates/paavo-proto/src/board.rs`: + +```rust + #[test] + fn parse_full_probe_rs_list_line() { + let line = "[0]: MCU-LINK on-board (r2E4) CMSIS-DAP V3.172 \ + -- 1fc9:0143-0:EDFHUAFM4J5ZJ (CMSIS-DAP)"; + let s = ProbeSelector::parse(line).unwrap(); + assert_eq!( + s, + ProbeSelector { + vid: "1fc9".into(), + pid: "0143".into(), + serial: "EDFHUAFM4J5ZJ".into(), + interface: Some(0), + } + ); + } +``` + +- [ ] **Step 2: Run — expect PASS (it should already pass)** + +Run: `cargo test -p paavo-proto board::tests::parse_full_probe_rs_list_line` +Expected: PASS. `extract_selector_token` finds the last ` -- `, strips ` (CMSIS-DAP)`, leaving `1fc9:0143-0:EDFHUAFM4J5ZJ`. + +> If it FAILS, the bug is in `extract_selector_token` from Task 2 — fix it there (it is the only code that handles the line form) before continuing. + +- [ ] **Step 3: Commit** + +```bash +git add crates/paavo-proto/src/board.rs +git commit -m "test(proto): ProbeSelector::parse accepts full probe-rs list line" +``` + +--- + +### Task 4: Wire `paavo-cli board add` to the parser + +**Files:** +- Modify: `crates/paavo-cli/src/cmd_boards.rs:34-56` +- Modify: `crates/paavo-cli/src/cli.rs:145` +- Test (create): `crates/paavo-cli/tests/board_add_selector.rs` + +- [ ] **Step 1: Write the failing CLI test (fail-fast, no daemon)** + +Create `crates/paavo-cli/tests/board_add_selector.rs`: + +```rust +//! `board add` must reject a malformed --probe locally, before any network +//! call. We point PAAVO_HOST at an unreachable address; if the command fails +//! with the parse message (not a connection error) we know it never POSTed. + +use assert_cmd::Command as AssertCommand; + +#[test] +fn board_add_rejects_invalid_probe_before_network() { + let mut cmd = AssertCommand::cargo_bin("paavo-cli").unwrap(); + cmd.env("PAAVO_HOST", "http://127.0.0.1:1") + .args([ + "board", "add", + "--kind", "mcxa266", + "--instance", "mcxa266-99", + "--probe", "zz:gg:NOSER", + "--chip", "MCXA266VFL", + "--target", "frdm-mcx-a266", + ]); + cmd.assert() + .failure() + .stderr(predicates::str::contains("bad VID")); +} +``` + +- [ ] **Step 2: Run — expect FAIL (today it splits naively and tries to POST)** + +Run: `cargo test -p paavo-cli --test board_add_selector` +Expected: FAIL — current code accepts `zz`/`gg` and attempts a POST, so stderr shows a connection error, not `"bad VID"`. + +- [ ] **Step 3: Replace the naive split with the parser** + +In `crates/paavo-cli/src/cmd_boards.rs`, replace the `BoardOp::Add` body (lines 34-56) — the `let mut parts = probe.split(':'); …` block through the `BoardSpec` literal — with: + +```rust + let probe_selector = ProbeSelector::parse(&probe).map_err(|e| { + anyhow!( + "invalid --probe {probe:?}: {e}\n\n\ + Paste a probe-rs selector (`1fc9:0143:SERIAL`) or a full \ + `probe-rs list` line." + ) + })?; + let spec = BoardSpec { + id: instance, + kind, + probe_selector, + chip_name: chip, + target_name: target, + wiring_profile: Some(wiring_profile), + health: BoardHealth::Healthy, + }; + client.add_board(&spec).await?; + println!("added: {}", spec.id); + Ok(()) +``` + +The existing `use paavo_proto::{BoardHealth, BoardSpec, ProbeSelector};` already imports `ProbeSelector`; `anyhow!` is already imported. + +- [ ] **Step 4: Reword the `--probe` help** + +In `crates/paavo-cli/src/cli.rs`, replace the `probe` doc comment (line 145, `/// VID:PID:serial.`) with: + +```rust + /// Probe selector. Accepts a probe-rs selector token + /// (`1fc9:0143:SERIAL`, optional `-IFACE`) or a full `probe-rs list` + /// line pasted verbatim. + #[arg(long)] + probe: String, +``` + +- [ ] **Step 5: Run — CLI test passes** + +Run: `cargo test -p paavo-cli --test board_add_selector` +Expected: PASS — `parse("zz:gg:NOSER")` returns `BadVid`, the command errors locally with `"bad VID"`, never contacting `127.0.0.1:1`. + +- [ ] **Step 6: Commit** + +```bash +git add crates/paavo-cli/src/cmd_boards.rs crates/paavo-cli/src/cli.rs crates/paavo-cli/tests/board_add_selector.rs +git commit -m "feat(paavo-cli): board add accepts probe-rs selector strings" +``` + +--- + +### Task 5: Validate the selector at `POST /boards` + +**Files:** +- Modify: `crates/paavod/src/routes/boards.rs:56-60` +- Test: `crates/paavod/tests/api_boards.rs` (append) + +- [ ] **Step 1: Write the failing daemon test** + +Append to `crates/paavod/tests/api_boards.rs` (reuses the file's existing `state()`, `sample_board_json()`, `post_json()` helpers): + +```rust +#[tokio::test] +async fn add_board_rejects_non_hex_vid() { + let app = build_router(state()); + let mut body = sample_board_json(); + body["probe_selector"]["vid"] = serde_json::json!("zz"); + let resp = post_json(app, "/boards", body).await; + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); +} +``` + +- [ ] **Step 2: Run — expect FAIL (no validation yet → 201)** + +Run: `cargo test -p paavod --test api_boards add_board_rejects_non_hex_vid` +Expected: FAIL — currently returns `201 Created` (the bad selector is stored). + +- [ ] **Step 3: Add validation in `add_board`** + +In `crates/paavod/src/routes/boards.rs::add_board`, after the `health == Healthy` check (immediately before `let now_ms = …` at line 56), insert: + +```rust + spec.probe_selector + .validate() + .map_err(|e| (StatusCode::BAD_REQUEST, format!("invalid probe_selector: {e}")))?; +``` + +(`StatusCode` is already imported in this file.) + +- [ ] **Step 4: Run — daemon test passes** + +Run: `cargo test -p paavod --test api_boards` +Expected: PASS — bad VID now yields `400`; existing `/boards` tests still green. + +- [ ] **Step 5: Commit** + +```bash +git add crates/paavod/src/routes/boards.rs crates/paavod/tests/api_boards.rs +git commit -m "feat(paavod): validate probe_selector at POST /boards (400 on bad hex)" +``` + +--- + +### Task 6: Thread the interface into probe-rs at attach + +Hardware-only code path (`RealSession::connect` runs only with a real probe), so this is verified by compilation + clippy; the `PAAVO_HW=1` tests exercise it on real hardware. + +**Files:** +- Modify: `crates/paavo-probe/src/session.rs:164` + +- [ ] **Step 1: Pass the interface through** + +In `crates/paavo-probe/src/session.rs`, change the `DebugProbeSelector` literal (line 164) from `interface: None,` to: + +```rust + interface: opts.probe_selector.interface, +``` + +So the block reads: + +```rust + let selector = DebugProbeSelector { + vendor_id: vid, + product_id: pid, + interface: opts.probe_selector.interface, + serial_number: serial_filter.clone(), + }; +``` + +- [ ] **Step 2: Build + lint** + +Run: `cargo clippy -p paavo-probe --all-targets -- -D warnings` +Expected: PASS (no warnings). + +- [ ] **Step 3: Commit** + +```bash +git add crates/paavo-probe/src/session.rs +git commit -m "feat(paavo-probe): honor ProbeSelector.interface at probe attach" +``` + +--- + +### Task 7: Docs + +**Files:** +- Modify: `docs/deployment.md` (the stale `board add`/`boards.toml` claim) +- Modify: `AGENTS.md` (landmines note) + +- [ ] **Step 1: Correct `docs/deployment.md`** + +Find the `boards.toml` bullet (under "State directory layout"): + +``` +- `boards.toml` — `paavo-cli board add` writes this; restart paavod to pick up changes. +``` + +Replace with: + +``` +- `boards.toml` — declarative seed read at startup (inserts boards whose id is + not already in the DB). `paavo-cli board add` writes the **DB** via `POST + /boards`, not this file; edit `boards.toml` by hand to seed a fresh deploy. +``` + +- [ ] **Step 2: Add an `AGENTS.md` landmine note** + +Under the "Landmines & gotchas" section in `AGENTS.md`, add a bullet: + +``` +- **`board add` accepts probe-rs `list` output directly.** `paavo-cli board add + --probe` takes a probe-rs selector token (`1fc9:0143-0:SERIAL`, where `-0` is + the USB interface) or a full pasted `probe-rs list` line. VID/PID are + validated as hex at registration (CLI **and** `POST /boards`), not deferred to + `probe_attach`. The canonical parser is `ProbeSelector::parse` in + `paavo-proto`. +``` + +- [ ] **Step 3: Commit** + +```bash +git add docs/deployment.md AGENTS.md +git commit -m "docs: board add parses probe-rs selectors; fix boards.toml description" +``` + +--- + +### Final verification (CI parity) + +- [ ] **Run the full gate** + +```bash +cargo fmt --all -- --check +cargo clippy --workspace --all-targets -- -D warnings +cargo test --workspace +``` +Expected: all green (`RUSTFLAGS="-Dwarnings"` in CI — no warnings allowed). + +- [ ] **If `cargo fmt --all -- --check` reports diffs**, run `cargo fmt --all`, review, and commit: + +```bash +git add -A +git commit -m "style: cargo fmt" +``` + +--- + +## Self-review + +**Spec coverage:** Goals → Task 4 (both input forms), Task 2/3 (grammar incl. colon-serials), Task 1+6 (preserve interface), Task 4+5 (fail-fast CLI + daemon), Task 2 (4-hex normalization). Non-goals respected (no discovery endpoint; serial stays `String`; `boards.toml` structured; `parse_hex_u16` duplicated, noted). Wire-compat (Task 1 tests), migration (none; covered in Task 1 note). Every spec "Affected files" entry has a task. + +**Placeholder scan:** none — all steps carry real code/commands and expected output. + +**Type consistency:** `ProbeSelector { vid, pid, serial, interface }` used identically across Tasks 1-6; `ProbeSelectorParseError::{Format,BadVid,BadPid,BadInterface}` defined in Task 2 and matched in Task 2's tests and surfaced in Task 4's CLI message; `validate()` defined in Task 2, called in Task 5; `parse()` defined in Task 2, called in Task 4; `extract_selector_token`/`parse_hex_u16` defined in Task 2, exercised in Task 3. diff --git a/docs/superpowers/specs/2026-06-29-probe-selector-parsing-design.md b/docs/superpowers/specs/2026-06-29-probe-selector-parsing-design.md new file mode 100644 index 0000000..d4314b2 --- /dev/null +++ b/docs/superpowers/specs/2026-06-29-probe-selector-parsing-design.md @@ -0,0 +1,282 @@ +# `paavo-cli board add`: accept probe-rs selector strings directly + +**Date:** 2026-06-29 +**Status:** Approved (design) +**Crates:** `paavo-proto` (parser + type), `paavo-cli` (wiring), `paavod` (validation), `paavo-probe` (attach plumbing) + +## Problem + +Registering a board today means hand-massaging the output of `probe-rs list` +into `paavo-cli board add --probe VID:PID:serial`, and the CLI parses that with a +naive split (`crates/paavo-cli/src/cmd_boards.rs:35`): + +```rust +let mut parts = probe.split(':'); +let vid = parts.next()...; let pid = parts.next()...; let serial = parts.next()...; +``` + +That is wrong in three ways against what `probe-rs list` actually prints: + +``` +[0]: MCU-LINK on-board (r2E4) CMSIS-DAP V3.172 -- 1fc9:0143-0:EDFHUAFM4J5ZJ (CMSIS-DAP) +``` + +1. **The `-0` is the USB *interface* index**, not part of the PID. probe-rs's + selector grammar is `VID:PID[-INTERFACE][:SERIAL]` (see + `probe-rs/src/probe/selector.rs::from_str`). The naive split stores + `pid = "0143-0"`, which then fails `u16::from_str_radix(..,16)`. +2. **Serials can contain colons.** probe-rs uses `splitn(3, ':')` precisely + because some serials are colon-bearing (ESP JTAG uses a MAC, e.g. + `303a:1001:DC:DA:0C:D3:FE:D8`). `split(':')` truncates them. +3. **Failure is discovered far too late.** VID/PID are stored as raw strings and + only parsed as hex at `probe_attach` (`crates/paavo-probe/src/session.rs:140`, + `:146`). A bad selector therefore survives registration, queues a job, builds + the crate, claims a board, and only *then* fails with `infra_err` — an entire + build+flash cycle wasted on a typo. + +Operators will copy-paste `probe-rs list` output. The tool should understand it. + +## Goals + +- `paavo-cli board add --probe ...` accepts **either** a bare selector token + (`1fc9:0143-0:EDFHUAFM4J5ZJ`) **or** a full pasted `probe-rs list` line, and + normalizes it. +- Match probe-rs's own grammar: `splitn(3, ':')`, hex VID/PID, optional + `-INTERFACE`, colon-bearing serials. +- **Preserve the USB interface** (the `-N`) so multi-interface probes (FTDI + dual-channel `0403:6010-1`) remain addressable. +- **Fail fast.** A malformed selector is rejected at `board add` (locally, before + the network round-trip) *and* at `POST /boards` (so any client, including + `curl`, gets a `400` instead of a row that fails at attach). +- Canonicalize stored VID/PID to lowercase 4-hex (`0X1FC9` → `1fc9`). + +## Non-goals (YAGNI) + +- **No daemon-side probe discovery.** A `paavo-cli board discover` that runs + `probe-rs list` on the lab box and offers a numbered pick is a worthwhile + *future* follow-up (no pasting at all), but it needs a new daemon endpoint that + pokes hardware plus its own auth story. Out of scope here; noted in + [Future work](#future-work-out-of-scope-here). +- **Serial stays a `String` (empty = "no filter").** probe-rs distinguishes "no + serial" (`None`) from "match probes with empty serial" (`Some("")`); paavo + collapses both to `""` (`crates/paavo-probe/src/session.rs:153`). Preserving + that distinction is extra wire surface for a case the lab's serial-bearing + CMSIS-DAP probes never hit. Left as-is. +- **No `boards.toml` grammar change.** `boards.toml` is structured TOML + (`vid = "1fc9"`, and now optionally `interface = 0`), not a probe-rs token. The + new parser is a `board add` convenience only; TOML authors get the optional + field for free via serde. +- **No consolidation of `paavo-probe::parse_hex_u16`** into the new proto helper. + Both will tolerate `0x`/whitespace and parse base-16 identically; deduping them + is a tidy-up that can come later. + +## Decisions + +| Question | Decision | +|----------|----------| +| Accepted input forms | Bare selector token **and** full `probe-rs list` line. | +| Where the parser lives | `paavo-proto` (`ProbeSelector::parse`) — it owns the type; both CLI and daemon depend on proto. | +| The `-INTERFACE` index | **Preserved** as a new optional `ProbeSelector::interface: Option`, threaded to probe-rs at attach. | +| Validation scope | **CLI + daemon.** CLI parses/normalizes for instant local feedback; `POST /boards` re-validates (hex VID/PID) → `400`. | +| VID/PID normalization | Lowercase 4-hex via `format!("{:04x}", u16)`; tolerate `0x`/whitespace. | +| Serial | `String`, may be empty, may contain `:` (`splitn(3, ':')`). | +| Wire compatibility | `ProbeSelector` has **no** `#[serde(deny_unknown_fields)]`, so the new field is additive both ways: old JSON → `None` (serde default); new JSON → old readers ignore it. No DB migration (JSON in a `TEXT` column). | +| Error type | `ProbeSelectorParseError` (thiserror) in proto; CLI wraps with `anyhow` context; daemon maps to `400`. | + +## Design + +### 1. Wire type — `crates/paavo-proto/src/board.rs` + +Add one optional field, mirroring the existing `BoardSelector` optional-field +style (`board.rs:55-59`): + +```rust +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ProbeSelector { + /// USB vendor id, normalized lowercase 4-hex, e.g. `"1fc9"`. + pub vid: String, + /// USB product id, normalized lowercase 4-hex, e.g. `"0143"`. + pub pid: String, + /// Probe serial number as reported by USB. May be empty ("no filter") + /// and may contain `:` (e.g. ESP JTAG MAC serials). + pub serial: String, + /// USB interface index (the `-N` in a probe-rs selector). `None` matches + /// any interface; set only for multi-interface probes. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub interface: Option, +} +``` + +`skip_serializing_if` keeps existing rows and the byte-level wire-compat fixtures +identical when `interface` is `None`. + +### 2. Parser — `crates/paavo-proto/src/board.rs` + +```rust +impl ProbeSelector { + /// Parse a probe-rs selector token OR a full `probe-rs list` line. + pub fn parse(input: &str) -> Result; + + /// Validate that vid/pid are hex u16 (used by paavod at registration). + pub fn validate(&self) -> Result<(), ProbeSelectorParseError>; +} +``` + +`parse` algorithm: + +1. **Extract the token.** Trim. If the input contains ` -- ` (a `probe-rs list` + line: `[N]: -- ()`), take the substring after + the **last** ` -- `, then strip a trailing ` (...)` parenthetical, then trim. + Otherwise use the trimmed input as the token. +2. **Split the token** with `splitn(3, ':')` → `vid`, `pid_iface`, `serial?` + (3-way split so colons inside the serial survive). +3. **Interface.** `pid_iface.split_once('-')`: if present, `pid` is the left + side and the right side is the interface — empty (the `VID:PID-` case) → + `None`, else `u8::from_str` → `Some`. If no `-`, `pid = pid_iface`, + `interface = None`. +4. **Hex VID/PID.** Parse each as `u16` base-16 (tolerating a `0x`/`0X` prefix + and surrounding whitespace, matching `paavo-probe`'s `parse_hex_u16`). + Re-emit normalized as `format!("{:04x}", n)`. +5. **Serial.** The third `splitn` part verbatim if present, else `""`. +6. Return `ProbeSelector { vid, pid, serial, interface }`. + +`validate` re-runs step 4's hex check on the struct's existing `vid`/`pid` +strings (the daemon receives an already-structured selector over the wire). + +```rust +#[derive(thiserror::Error, Debug)] +pub enum ProbeSelectorParseError { + #[error("selector is empty or missing the PID (expected `VID:PID[-IFACE][:SERIAL]`, \ + e.g. `1fc9:0143:ABCD1234`)")] + Format, + #[error("bad VID {value:?}: {source}")] + BadVid { value: String, source: std::num::ParseIntError }, + #[error("bad PID {value:?}: {source}")] + BadPid { value: String, source: std::num::ParseIntError }, + #[error("bad USB interface {value:?}: {source}")] + BadInterface { value: String, source: std::num::ParseIntError }, +} +``` + +### 3. CLI wiring — `crates/paavo-cli/src/cmd_boards.rs` + +Replace the `probe.split(':')` block (`cmd_boards.rs:35-47`) with the proto +parser, failing locally before any network call: + +```rust +let probe_selector = ProbeSelector::parse(&probe).map_err(|e| { + anyhow!( + "invalid --probe {probe:?}: {e}\n\n\ + Paste a probe-rs selector (`1fc9:0143:SERIAL`) or a full `probe-rs list` line." + ) +})?; +let spec = BoardSpec { id: instance, kind, probe_selector, chip_name: chip, + target_name: target, wiring_profile: Some(wiring_profile), + health: BoardHealth::Healthy }; +``` + +Update the `--probe` help (`crates/paavo-cli/src/cli.rs:145`) from `VID:PID:serial` +to note both accepted forms. + +### 4. Daemon validation — `crates/paavod/src/routes/boards.rs::add_board` + +After the existing `health == Healthy` check and before insert +(`routes/boards.rs:56-60`): + +```rust +spec.probe_selector + .validate() + .map_err(|e| (StatusCode::BAD_REQUEST, format!("invalid probe_selector: {e}")))?; +``` + +So a raw `curl` POST with a non-hex VID/PID is rejected at registration, not at +attach. + +### 5. Attach plumbing — `crates/paavo-probe/src/session.rs` + +Where the probe-rs `DebugProbeSelector` is constructed (`session.rs:161`), pass +the interface through instead of the implicit/`None` it uses today: + +```rust +let selector = DebugProbeSelector { + vendor_id: vid, + product_id: pid, + serial_number: serial_filter, + interface: opts.probe_selector.interface, +}; +``` + +VID/PID/serial plumbing is unchanged; `parse_hex_u16` still runs (and now always +succeeds, because registration validated). + +### 6. Existing data / migration + +- No schema migration: `ProbeSelector` is JSON in the `board.probe_selector` + `TEXT` column (`crates/paavo-db/migrations/V1__initial.sql:11`). Old rows + deserialize with `interface = None` via serde default. +- The previously hand-fixed `mcxa266-01` row (`pid = "0143"`, no interface) + remains valid; no backfill needed. + +## Testing / verification + +### `paavo-proto` unit tests (`#[cfg(test)]` in `board.rs`) + +`parse` cases: + +1. `bare_token` — `1fc9:0143:EDFHUAFM4J5ZJ` → vid/pid/serial, `interface == None`. +2. `with_interface` — `1fc9:0143-0:EDFHUAFM4J5ZJ` → `interface == Some(0)`. +3. `empty_interface` — `1fc9:0143-:S` → `interface == None`. +4. `colon_serial` — `303a:1001:DC:DA:0C:D3:FE:D8` → serial kept whole. +5. `no_serial` — `1fc9:0143` → serial `""`, `interface == None`. +6. `full_list_line` — `[0]: MCU-LINK on-board (r2E4) CMSIS-DAP V3.172 -- 1fc9:0143-0:EDFHUAFM4J5ZJ (CMSIS-DAP)` + → `{ vid:"1fc9", pid:"0143", serial:"EDFHUAFM4J5ZJ", interface:Some(0) }`. +7. `normalization` — `0X1FC9:143` and ` 1fc9 : 0143 ` → `vid:"1fc9", pid:"0143"`. +8. errors — bad hex VID (`zz:0143:S` → `BadVid`), bad PID (`1fc9:gg:S` → `BadPid`), + missing PID (`1fc9` → `Format`), bad interface (`1fc9:0143-x:S` → `BadInterface`). + +Wire-compat: + +9. `roundtrip_none` — `ProbeSelector` with `interface: None` serializes to the + pre-existing JSON byte-for-byte (extend/keep the existing fixture). +10. `roundtrip_some` — new fixture: `interface: Some(0)` adds exactly the + `"interface":0` key and round-trips. + +### `paavod` (`crates/paavod/...` route test) + +- `add_board_rejects_bad_selector` — `POST /boards` with `vid:"zz"` ⇒ `400`. + +### `paavo-cli` (`crates/paavo-cli/tests/`, `assert_cmd` + `predicates`) + +- `board_add_accepts_full_list_line` — `board add --probe '[0]: ... -- 1fc9:0143-0:SER (CMSIS-DAP)' ...` + against the existing fake-daemon harness registers a board whose selector is + `{ vid:"1fc9", pid:"0143", serial:"SER", interface:Some(0) }`. +- `board_add_rejects_garbage_probe` — a non-hex `--probe` exits non-zero with the + helpful message, **without** contacting the daemon. + +### Full gate (CI parity, from `AGENTS.md`) + +```bash +cargo fmt --all -- --check +cargo clippy --workspace --all-targets -- -D warnings +cargo test --workspace +``` + +All green (`RUSTFLAGS="-Dwarnings"` — no warnings allowed). + +## Affected files + +- `crates/paavo-proto/src/board.rs` — `interface` field; `ProbeSelector::parse` + + `validate`; `ProbeSelectorParseError`; unit tests + wire-compat fixtures. +- `crates/paavo-cli/src/cmd_boards.rs` — use `ProbeSelector::parse`. +- `crates/paavo-cli/src/cli.rs` — reword `--probe` help. +- `crates/paavod/src/routes/boards.rs` — `validate()` → `400` in `add_board`. +- `crates/paavo-probe/src/session.rs` — pass `interface` into `DebugProbeSelector`. +- `crates/paavo-cli/tests/…` — CLI accept/reject tests. + +## Future work (out of scope here) + +1. **`paavo-cli board discover`** — paavod runs `probe-rs list` on the lab box and + returns a numbered inventory; the operator picks an index, no pasting. Needs a + new daemon endpoint that touches hardware plus an auth story for it. +2. **Consolidate hex parsing** — fold `paavo-probe::parse_hex_u16` into the + `paavo-proto` helper so there is a single base-16 VID/PID parser.