Skip to content

Fix probe-rs audit follow-ups#1005

Merged
zackees merged 1 commit into
mainfrom
fix/1002-probe-rs-audit
Jul 8, 2026
Merged

Fix probe-rs audit follow-ups#1005
zackees merged 1 commit into
mainfrom
fix/1002-probe-rs-audit

Conversation

@zackees

@zackees zackees commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

  • expose no_probe_rs on the MCP trigger_deploy schema and forward it into deploy requests
  • stage managed probe-rs installs through a sibling temp file before rename
  • clean up the managed probe-rs staging directory after install attempts

Closes #1002

Local validation

  • soldr cargo test -p fbuild-deploy probe_rs -> 9 passed
  • soldr cargo test -p fbuild-deploy lpc -> 33 passed
  • soldr --no-cache cargo test -p fbuild-cli trigger_deploy_exposes_probe_rs_opt_out -> 1 passed
  • soldr --no-cache cargo test -p fbuild-cli bool_arg_reads_boolean_or_default -> 1 passed
  • soldr cargo fmt --all --check
  • git diff --check
  • soldr --no-cache cargo build -p fbuild-cli -p fbuild-daemon with isolated temp CARGO_TARGET_DIR

Hardware validation

  • fbuild port scan saw COM10 as 1FC9:0132 / NXP LPC-Link2 CMSIS-DAP, serial 0B03400A
  • FBUILD_DEV_MODE=1 <temp-target>\fbuild.exe deploy tests/platform/lpc845brk -e lpc845brk --port COM10 --verbose -> deploy succeeded (full flash)
  • daemon log confirmed SWD path: attempting SWD flash via probe-rs ... probe=1fc9:0132
  • post-deploy fbuild port scan still saw COM10 as 1FC9:0132

Remote Actions wait intentionally skipped per maintainer instruction; this PR was locally built and hardware-validated on this machine.

Summary by CodeRabbit

  • New Features

    • Added an option to force deploys through a specific LPC flashing method.
    • Improved deploy input handling so boolean options are parsed consistently.
  • Bug Fixes

    • Made probe-rs installs safer by cleaning up temporary staging files more reliably.
    • Updated binary replacement to use an atomic swap, reducing the chance of partial or conflicting installs.
    • Added coverage for boolean defaults and install cleanup behavior.

@zackees
zackees merged commit 7098660 into main Jul 8, 2026
89 of 93 checks passed
@zackees
zackees deleted the fix/1002-probe-rs-audit branch July 8, 2026 18:37
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0d184349-5f96-4f9c-a236-76585b9081a7

📥 Commits

Reviewing files that changed from the base of the PR and between 67802fe and acf02c6.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • crates/fbuild-cli/src/mcp/definitions.rs
  • crates/fbuild-cli/src/mcp/tools.rs
  • crates/fbuild-deploy/Cargo.toml
  • crates/fbuild-deploy/src/probe_rs.rs

📝 Walkthrough

Walkthrough

This PR adds a no_probe_rs boolean option to the MCP trigger_deploy tool schema and argument parsing via a new bool_arg helper, and reworks the probe-rs installer to stage binaries to unique temp files with atomic rename and staging-directory cleanup, plus new tests and a zip dev-dependency.

Changes

no_probe_rs MCP flag and probe-rs install hardening

Layer / File(s) Summary
Expose no_probe_rs schema option
crates/fbuild-cli/src/mcp/definitions.rs
Adds no_probe_rs boolean property with description and default: false to the trigger_deploy input schema, plus a test asserting its type and default.
Parse no_probe_rs argument via shared helper
crates/fbuild-cli/src/mcp/tools.rs
Adds a private bool_arg helper for reading boolean args with a default, uses it for no_probe_rs, wires the parsed value into DeployRequest, and adds a unit test for the helper.
Atomic probe-rs binary staging and install
crates/fbuild-deploy/src/probe_rs.rs, crates/fbuild-deploy/Cargo.toml
Adds an atomic counter for unique temp filenames, a probe_rs_temp_install_path builder, and reworks extract_and_install_probe_rs to stage the binary to a temp file, set Unix executable permissions, and atomically rename it into place, removing the temp file on failure; adds zip as a dev-dependency and a test confirming existing binaries are replaced with no leftover temp files.
Staging directory cleanup
crates/fbuild-deploy/src/probe_rs.rs
Adds cleanup_probe_rs_staging_dir to asynchronously remove the staging tree (ignoring missing-directory errors, warning on others), wires it into install_managed_probe_rs, and adds a test verifying idempotent cleanup.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • FastLED/fbuild#1001: Adds/propagates the same no_probe_rs deploy flag through the deploy request and MCP trigger_deploy tooling, overlapping directly with the request/dispatch code paths touched here.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1002-probe-rs-audit

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

[META] probe-rs + LPC deploy audit: reconcile follow-ups after #1000

1 participant