feat(debug): add fbuild debug verb with ESP32 native-gdbstub orchestration (#1144) - #1206
Conversation
…ation (#1144) - Capability matrix as code: ESP32 family => native IDF/ROM gdbstub orchestration; CH32V => first-class "planned, pending FastLED/soundwave#38" message; AVR => honestly unsupported (no trap architecture). Unsupported/planned targets exit cleanly with explanatory output, never a stack trace. - fbuild debug [dir] [-e env] [--no-flash] [--port]: build+deploy via the existing deploy path, resolve the exact build's ELF (fbuild symbols discovery), resolve the toolchain gdb (build_info cc_path gcc->gdb swap, fallback PATH search via the mcu's toolchain prefix), resolve the CDC port (daemon device list; --port override; Windows \.\COMx rewriting), launch gdb interactively with target remote. - Honest sdkconfig story: gdbstub-on-panic (CONFIG_ESP_SYSTEM_PANIC_ GDBSTUB) cannot yet be injected by fbuild (docs/sdkconfig.md is a design proposal); the command reports the current panic mode from the project's sdkconfig and names the build_flags escape hatch. No faked injection. Part of #1144 (ESP32 arm + capability matrix; CH32V stub and fbuild crashdump remain, gated on FastLED/soundwave#38). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds ChangesDebug command
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant fbuild_debug as fbuild debug
participant deploy_run_deploy as deploy::run_deploy
participant Daemon
participant GDB
User->>fbuild_debug: invoke command
fbuild_debug->>deploy_run_deploy: build and flash unless --no-flash
fbuild_debug->>Daemon: discover CDC device port
fbuild_debug->>GDB: launch with ELF, baud, and target remote
GDB-->>User: interactive debug session
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fbuild-cli/src/cli/debug.rs`:
- Around line 536-539: Update the gdb launch in the surrounding debug command
flow to use the sanctioned subprocess wrapper; if the interactive session
requires direct Command spawning, add the required allow-direct-spawn marker
with a concise reason that inherited stdio is necessary. Preserve the existing
arguments, status handling, and launch error mapping.
- Around line 352-360: Update resolve_debug_elf and its run_debug caller to
resolve the ELF from the selected env_name/platform/mcu build output rather than
using unfiltered discover_elf_in_project(project_dir). Prefer the selected
environment’s build_info_<env>.json prog_path when available, and ensure the ELF
passed to gdb matches the environment used for toolchain and gdb resolution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e191269f-b494-4522-83f2-3c226768f6e2
📒 Files selected for processing (6)
agents/docs/commands-reference.mdcrates/fbuild-cli/src/cli/args.rscrates/fbuild-cli/src/cli/debug.rscrates/fbuild-cli/src/cli/dispatch.rscrates/fbuild-cli/src/cli/mod.rsdocs/reference/cli.md
| fn resolve_debug_elf(project_dir: &Path) -> Result<PathBuf> { | ||
| discover_elf_in_project(project_dir).ok_or_else(|| { | ||
| FbuildError::Other(format!( | ||
| "no ELF found under {} — run `fbuild debug` without --no-flash to build one, \ | ||
| or run `fbuild build` first", | ||
| project_dir.display() | ||
| )) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether discover_elf_in_project can disambiguate by environment.
ast-grep outline crates/fbuild-build/src --match discover_elf_in_project --items all
rg -n -A15 'fn discover_elf_in_project' --type=rustRepository: FastLED/fbuild
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files matching debug/discover/elf:"
git ls-files | rg '(^|/)(debug|.*Elf|.*elf|.*build).*\.rs$|crates/fbuild-cli/src/cli/debug\.rs|crates/fbuild-build/src' | head -200
echo
echo "Find discover_elf_in_project text:"
rg -n "discover_elf_in_project|resolve_debug_elf|resolve_debug_target|no-flash|--no-flash|environment|mcu|platform|env_name" -S --glob '*.rs' crates | head -300
echo
echo "Outline crates/fbuild-cli/src/cli/debug.rs symbols:"
ast-grep outline crates/fbuild-cli/src/cli/debug.rs --view expanded | head -200 || trueRepository: FastLED/fbuild
Length of output: 43039
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "debug.rs relevant sections:"
cat -n crates/fbuild-cli/src/cli/debug.rs | sed -n '280,430p'
echo
cat -n crates/fbuild-cli/src/cli/debug.rs | sed -n '430,535p'
echo
cat -n crates/fbuild-cli/src/cli/debug.rs | sed -n '720,805p'
echo
echo "Search for Elf discovery symbols/paths:"
rg -n "firmware\.elf|build_dir|env_name|ProjectDir|ProjectDirResolver|Environment|GetBuildDir|get_build_dir|build.*elf|discover.*elf|glob" -S --glob '*.rs' crates | head -400
echo
echo "Candidate symbols that may implement ELF discovery/build dir selection:"
git ls-files '*.rs' | xargs rg -n "struct ProjectDirResolver|impl ProjectDirResolver|fn get_build_dir|get_build_dir|pub .*BuildDir|BuildDir|BuildDir" --type=rust | head -200Repository: FastLED/fbuild
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "imports and discover symbol in debug.rs:"
cat -n crates/fbuild-cli/src/cli/debug.rs | sed -n '1,120p'
echo
rg -n "discover_elf_in_project|use .*discover|from .*discover|build.*info|find_build_info_near|load_build_info|glob|WalkDir|walk|firmware\.elf" crates/fbuild-cli/src/cli/debug.rs
echo
echo "symbols with exact target function names:"
rg -n "fn find_build_info_near|fn load_build_info|fn discover_elf_in_project|struct BuildInfo|fn load_build" crates --type=rust -S
echo
echo "candidate crates with relevant names:"
git ls-files '*discover*;*elf*;*.rs' | rg 'fbuild-(core|paths|cli|packages|build|symbols|config)' | head -200Repository: FastLED/fbuild
Length of output: 9359
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "fbuild-build-engine symbol_analyzer discover_elf_in_project:"
cat -n crates/fbuild-build-engine/src/symbol_analyzer/mod.rs | sed -n '500,600p'
echo
echo "imports/exports around symbol_analyzer:"
cat -n crates/fbuild-build-engine/src/symbol_analyzer/mod.rs | sed -n '1,80p'
echo "imports/exports around symbol_analyzer in engine lib:"
rg -n "mod symbol_analyzer|pub use|symbol_analyzer" crates/fbuild-build-engine/src/lib.rs crates/fbuild-build/src/lib.rs -S
echo
echo "cargo deps for fbuild-core/paths if manifest exports them:"
rg -n "name = \"fbuild-(core|paths|packages-fetch|packages)\"|path = \"fbuild-(core|paths|packages-fetch|packages)\"" crates/Cargo.toml Cargo.toml --type=toml -SRepository: FastLED/fbuild
Length of output: 144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
time cat -n crates/fbuild-build-engine/src/symbol_analyzer/mod.rs | sed -n '500,580p'Repository: FastLED/fbuild
Length of output: 3904
Resolve the ELF relative to the selected debug environment.
run_debug selects a specific env_name/platform/mcu, but resolve_debug_elf(project_dir) uses the unfiltered discovery fallback, so with multiple built environments it can hand gdb a stale ELF from another env while later resolving the requested toolchain/gdb. Scope ELF lookup to the resolved environment build output, or load prog_path from that environment’s build_info_<env>.json.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/fbuild-cli/src/cli/debug.rs` around lines 352 - 360, Update
resolve_debug_elf and its run_debug caller to resolve the ELF from the selected
env_name/platform/mcu build output rather than using unfiltered
discover_elf_in_project(project_dir). Prefer the selected environment’s
build_info_<env>.json prog_path when available, and ensure the ELF passed to gdb
matches the environment used for toolchain and gdb resolution.
| let status = Command::new(&gdb_path) | ||
| .args(&argv) | ||
| .status() | ||
| .map_err(|e| FbuildError::Other(format!("failed to launch {}: {e}", gdb_path.display())))?; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate the sanctioned subprocess wrapper API and its async-safety.
rg -n -C5 'pub fn run_command_passthrough|pub async fn run_command_passthrough|pub fn run_command\b' --type=rust
rg -n 'allow-direct-spawn' --type=rust -C2Repository: FastLED/fbuild
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -u
echo "== files matching debug.rs =="
fd -a 'debug\.rs$' . | sed 's#^\./##'
echo "== repository rust subprocess-related text broad =="
rg -n 'subprocess|run_command|direct.spawn|allow-direct|Command::new|status\(\)' --type=rust -C2 | head -n 300
echo "== git diff stat/name-only if available =="
git diff --stat || true
git diff -- crates/fbuild-cli/src/cli/debug.rs 2>/dev/null | sed -n '1,220p' || true
echo "== target outline =="
ast-grep outline crates/fbuild-cli/src/cli/debug.rs --match run_debug --view expanded || trueRepository: FastLED/fbuild
Length of output: 346
Route gdb launching through the sanctioned subprocess wrapper or mark it explicitly.
crates/fbuild-cli/src/cli/debug.rs currently uses a direct Command::new(&gdb_path).status() spawn with no allow-direct-spawn: <reason> marker, which conflicts with the Rust build-pipeline directive for non-diagnostic commands. Since this is an interactive gdb session, either add an in-line marker explaining inherited stdio is required, or use the sanctioned subprocess interface so CI/lint can see the bypass is intentional.
🧰 Tools
🪛 GitHub Actions: Lint subprocess spawns / 0_Lint subprocess spawns.txt
[error] 536-536: Direct subprocess spawn detected without an allow-direct-spawn: <reason> marker. Please route via fbuild_core::subprocess::{run_command,run_command_passthrough} or annotate with a one-line reason (see #141).
🪛 GitHub Actions: Lint subprocess spawns / Lint subprocess spawns
[error] 536-536: CI check failed (find_direct_subprocess.py). NEW direct spawn without an allow-direct-spawn: <reason> marker: uses Command::new(&gdb_path).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/fbuild-cli/src/cli/debug.rs` around lines 536 - 539, Update the gdb
launch in the surrounding debug command flow to use the sanctioned subprocess
wrapper; if the interactive session requires direct Command spawning, add the
required allow-direct-spawn marker with a concise reason that inherited stdio is
necessary. Preserve the existing arguments, status handling, and launch error
mapping.
Source: Pipeline failures
… unused_mut, rustdoc links main is red on six checks. They are unrelated to each other and accumulated because the workflows that run them are `pull_request`-triggered, so nothing re-ran them on main after the merges that broke them (#1197, #1206, #1207). Found while verifying an unrelated dependency bump (#1215); every failure there reproduced identically on main. Dylint: drop `crates/fbuild-cli/src/cli/clangd_config.rs` from ban_unrooted_tempdir's allowlist -- the file was deleted in #1197. Lint subprocess spawns: annotate three deliberate direct spawns with `allow-direct-spawn:` markers (clangd test driver, interactive gdb that must inherit terminal stdio, detached editor launch that must outlive the CLI). The scanner only inspects the hit line and the one line directly above it, so the marker must be a single line -- a wrapped comment silently fails to register. This also fixes Check (ubuntu-latest), which runs the same script. Check (macos-latest): `ports.rs` bound `let mut ports` unconditionally while only `cfg(target_os = "linux")` mutates it, so every other unix target tripped `-D unused-mut`. The `mut` now exists only inside the Linux cfg via a shadowing block, rather than an `#[allow]`. Documentation: six rustdoc errors across three crates. URL templates such as `?query=<optional filter>` parsed as unclosed HTML tags; several intra-doc links pointed at `cfg(target_os = "linux")` items that do not exist to resolve against when docs build on a non-Linux host; three more pointed at private items. Unresolvable links become code spans, each with a short note on why it is deliberately not a link. CH32V006 (`build / build`) is also red on main but is a vendor-C compile error in the openwch Arduino core, not CI hygiene, and is left for a targeted fix. Verified locally on Windows, all exit 0: check_dylint_allowlists.py, find_direct_subprocess.py --fail, cargo doc with RUSTDOCFLAGS=-D warnings, clippy -D warnings, cargo fmt --check, and `bash test` (77 suites, 0 failed). The macOS fix could not be verified locally: cross-compiling fbuild-serial to x86_64-apple-darwin fails on ring's build script needing a macOS `cc`. The cfg-shadowing pattern was verified in an isolated cargo project (clean under -D warnings with the cfg both on and off); Check (macos-latest) on this PR is the real proof. Co-Authored-By: Claude <noreply@anthropic.com>
Pinning dylint back to =6.0.1 made the driver build again, so the lints actually ran for the first time in a while and reported 34 violations across three crates. None are new: they accumulated in #1197, #1206 and #1207 while Dylint was failing at earlier steps (a stale allowlist path, then the 6.0.2 driver) and never reached the lint pass. Handled by category rather than uniformly. ban_std_pathbuf (30) -- migrated to `fbuild_core::path::NormalizedPath` in `fbuild-cli` (`ide.rs`, `debug.rs`, `ide_debug.rs`, `clangd_config/{mod, vscode,zed}.rs`), `fbuild-daemon` (`handlers/libraries.rs`) and the `sysfs_usb.rs` test fixtures. Not allowlisted: that allowlist's header states "New files MUST NOT be added here. The target state is zero entries", and exempting brand-new files would hollow out the gate that was just repaired. `NormalizedPath` derefs to `Path`, so call sites taking `&Path` are unchanged; `to_path_buf()` appears only where an external API (`BuildLayout::new`) demands an owned `PathBuf`. ban_std_fs_in_async (2) -- `handlers/libraries.rs::installed_dir_names` now uses async `fbuild_core::fs::read_dir` (the sanctioned re-export of `tokio::fs`), which makes it and `build_library_entries` async along with their three unit tests. It also switches the directory test from `Path::is_dir()` to `entry.file_type().await`: `is_dir()` is a blocking `stat` on the async worker, so keeping it would have satisfied the lint's letter while preserving exactly the stall the lint exists to prevent (#844). ban_raw_subprocess (2) -- allowlisted with justifications, which is what the lint's own diagnostic offers for genuinely-justified sites. Both are correct as raw spawns: `fbuild debug` hands the terminal to gdb, so the child must inherit this process's stdio rather than be captured; and `fbuild ide` launches the editor detached so it OUTLIVES the CLI, which is the opposite of what a containment group guarantees. Verified: `cargo clippy --workspace --all-targets -- -D warnings`, `cargo fmt --all --check`, and `bash test` (77 suites, 0 failed) all green. Dylint itself could not be run locally to pre-verify -- the pinned nightly and `cargo-dylint` are present, but `dylint-link` fails to link the lint cdylibs on this Windows host. CI is the proof for that gate. Co-Authored-By: Claude <noreply@anthropic.com>
Pinning dylint back to =6.0.1 made the driver build again, so the lints actually ran for the first time in a while and reported 34 violations across three crates. None are new: they accumulated in #1197, #1206 and #1207 while Dylint was failing at earlier steps (a stale allowlist path, then the 6.0.2 driver) and never reached the lint pass. Handled by category rather than uniformly. ban_std_pathbuf (30) -- migrated to `fbuild_core::path::NormalizedPath` in `fbuild-cli` (`ide.rs`, `debug.rs`, `ide_debug.rs`, `clangd_config/{mod, vscode,zed}.rs`), `fbuild-daemon` (`handlers/libraries.rs`) and the `sysfs_usb.rs` test fixtures. Not allowlisted: that allowlist's header states "New files MUST NOT be added here. The target state is zero entries", and exempting brand-new files would hollow out the gate that was just repaired. `NormalizedPath` derefs to `Path`, so call sites taking `&Path` are unchanged; `to_path_buf()` appears only where an external API (`BuildLayout::new`) demands an owned `PathBuf`. ban_std_fs_in_async (2) -- `handlers/libraries.rs::installed_dir_names` now uses async `fbuild_core::fs::read_dir` (the sanctioned re-export of `tokio::fs`), which makes it and `build_library_entries` async along with their three unit tests. It also switches the directory test from `Path::is_dir()` to `entry.file_type().await`: `is_dir()` is a blocking `stat` on the async worker, so keeping it would have satisfied the lint's letter while preserving exactly the stall the lint exists to prevent (#844). ban_raw_subprocess (2) -- allowlisted with justifications, which is what the lint's own diagnostic offers for genuinely-justified sites. Both are correct as raw spawns: `fbuild debug` hands the terminal to gdb, so the child must inherit this process's stdio rather than be captured; and `fbuild ide` launches the editor detached so it OUTLIVES the CLI, which is the opposite of what a containment group guarantees. Verified: `cargo clippy --workspace --all-targets -- -D warnings`, `cargo fmt --all --check`, and `bash test` (77 suites, 0 failed) all green. Dylint itself could not be run locally to pre-verify -- the pinned nightly and `cargo-dylint` are present, but `dylint-link` fails to link the lint cdylibs on this Windows host. CI is the proof for that gate. Co-Authored-By: Claude <noreply@anthropic.com> Also widens the cargo-cache .gitignore entries to `**/`-prefixed forms. The existing patterns contain a slash, so git anchors them to the repo root and they never covered nested crates; running any cargo command inside `dylints/<lint>/` (each is its own workspace) materializes a full registry cache there that a `git add -A` will stage.
Pinning dylint back to =6.0.1 made the driver build again, so the lints actually ran for the first time in a while and reported 34 violations across three crates. None are new: they accumulated in #1197, #1206 and #1207 while Dylint was failing at earlier steps (a stale allowlist path, then the 6.0.2 driver) and never reached the lint pass. Handled by category rather than uniformly. ban_std_pathbuf (30) -- migrated to `fbuild_core::path::NormalizedPath` in `fbuild-cli` (`ide.rs`, `debug.rs`, `ide_debug.rs`, `clangd_config/{mod, vscode,zed}.rs`), `fbuild-daemon` (`handlers/libraries.rs`) and the `sysfs_usb.rs` test fixtures. Not allowlisted: that allowlist's header states "New files MUST NOT be added here. The target state is zero entries", and exempting brand-new files would hollow out the gate that was just repaired. `NormalizedPath` derefs to `Path`, so call sites taking `&Path` are unchanged; `to_path_buf()` appears only where an external API (`BuildLayout::new`) demands an owned `PathBuf`. ban_std_fs_in_async (2) -- `handlers/libraries.rs::installed_dir_names` now uses async `fbuild_core::fs::read_dir` (the sanctioned re-export of `tokio::fs`), which makes it and `build_library_entries` async along with their three unit tests. It also switches the directory test from `Path::is_dir()` to `entry.file_type().await`: `is_dir()` is a blocking `stat` on the async worker, so keeping it would have satisfied the lint's letter while preserving exactly the stall the lint exists to prevent (#844). ban_raw_subprocess (2) -- allowlisted with justifications, which is what the lint's own diagnostic offers for genuinely-justified sites. Both are correct as raw spawns: `fbuild debug` hands the terminal to gdb, so the child must inherit this process's stdio rather than be captured; and `fbuild ide` launches the editor detached so it OUTLIVES the CLI, which is the opposite of what a containment group guarantees. Verified: `cargo clippy --workspace --all-targets -- -D warnings`, `cargo fmt --all --check`, and `bash test` (77 suites, 0 failed) all green. Dylint itself could not be run locally to pre-verify -- the pinned nightly and `cargo-dylint` are present, but `dylint-link` fails to link the lint cdylibs on this Windows host. CI is the proof for that gate. Co-Authored-By: Claude <noreply@anthropic.com> Also widens the cargo-cache .gitignore entries to `**/`-prefixed forms. The existing patterns contain a slash, so git anchors them to the repo root and they never covered nested crates; running any cargo command inside `dylints/<lint>/` (each is its own workspace) materializes a full registry cache there that a `git add -A` will stage.
Implements the "high value, low effort" arm of #1144 — the
fbuild debugverb with the per-target capability matrix and ESP32 native-gdbstub orchestration. The CH32V injected stub andfbuild crashdumpremain on the issue (both define their contract jointly with FastLED/soundwave#38).--no-flashto attach to what's running) → ELF resolution reusingfbuild symbols' discovery (build_info.json→ build dirs → loose*.elf) → gdb resolution (derive from the build's owncc_pathby gcc→gdb swap, else PATH candidates from the MCU's toolchain prefix) → CDC port from the daemon device list (--portoverride; auto-pick only when unambiguous; Windows\.\COMxrewriting for gdb) → interactivegdb <elf> -ex "target remote <port>".CONFIG_ESP_SYSTEM_PANIC_GDBSTUB(docs/sdkconfig.mdis a design proposal, not implemented) — the command reads the project's actual sdkconfig, reports the current panic mode, and names the existingbuild_flagsescape hatch instead of faking injection.Validation (local)
soldr cargo test -p fbuild-cli→ 262 passed (26 new: matrix, readiness note, gdb derivation/candidates over fake toolchain layouts, argv construction, COM rewriting, ELF fixtures, env/mcu resolution); clippy-D warningsclean; fmt clean; fullbash testbefore merge. The interactive gdb spawn path is deliberately untested (documented — nothing to assert without hardware).Part of #1144. Hardware validation (real ESP32 attach) noted in the docs as the remaining proof step.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
fbuild debugcommand for interactive GDB debugging on supported ESP32 devices.--no-flash,--port, project directory, and environment options.Documentation