fix(unwrap): production unwrap sweep + ban_unwrap_in_production rename (#844 item 11)

[zackees]

Summary

Closes#844 item 11. After accurate filtering (excluding #[cfg(test)] mod blocks and sibling test files like tests.rs/*_tests.rs), the production unwrap count was ~30 — not 2,238. All migrated, lint renamed and widened.

Migrations (28 sites across 21 files)

Per-file approach:

• fbuild-python/serial_monitor.rs (12): Mutex::lock → unwrap_or_else(|e| e.into_inner()) for poison tolerance; serde to_string → expect("ClientMessage X serialization is infallible")
• fbuild-serial/crash_decoder.rs (5): static regex compiles → expect("static <name> pattern is a valid regex")
• fbuild-deploy/esp32/image.rs (3): 4-byte slice converts → expect("4-byte slice converts to [u8; 4] (bounds checked above)")
• fbuild-daemon/handlers/websockets.rs (3): SerialServerMessage serde → expect("infallible")
• fbuild-config/bin/enrich_boards.rs (3): path utilities → expect("board JSON path always has a file stem")
• fbuild-python/async_serial_monitor.rs (2): ClientMessage serde → expect("infallible")
• fbuild-header-scan/scanner.rs (2): parser invariants → expect("is_raw_string_open guarantees ' ' / '(' ahead")
• fbuild-build/compiler.rs (2): COMPILER_IDENTITY_CACHE.lock() → poison-tolerant
• fbuild-build/pipeline/compile.rs (2): same poison-tolerant pattern
• fbuild-cli/cli/clang_tools.rs (2): semaphore expect("never closed before all tasks finish")
• fbuild-daemon/main.rs (2): static CORS origin → expect("'http://localhost' is a valid CORS origin")
• 1 site converted to ? (deploy pipeline closure already returns Result)

Lint extension

dylints/ban_unwrap_in_daemon_handlers/ renamed to dylints/ban_unwrap_in_production/:

• Scope widened from crates/fbuild-daemon/src/handlers/** to ALL of crates/fbuild-daemon/src/** AND crates/fbuild-cli/src/cli/**
• Sibling-test detection tightened from substring name.contains("tests") to strict patterns tests.rs / *_tests.rs / tests_*.rs (the old substring match would have accidentally exempted production files like run_tests_pipeline.rs)
• Workspace Cargo.toml exclude, ci/hooks/crate_guard.py allowlist, dylints/README.md row, and cross-references all updated

Test plan

• soldr cargo check --workspace --all-targets — clean
• soldr cargo clippy --workspace --all-targets -- -D warnings — clean
• soldr cargo test --workspace --no-fail-fast --lib — passes for every touched crate. The 3 pre-existing fbuild-daemonhandlers::{health,locks}::tests::... failures (panic at status_manager.rs:221: "can call blocking only when running on the multi-threaded runtime") reproduce identically on origin/main and are unrelated to this change.

Closes#844.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Expanded the custom lint to cover broader production code, not just daemon handlers.
  • Added guidance for replacing unsafe .unwrap() usage with safer alternatives and clearer error handling.

• Bug Fixes

  • Improved resilience across build, daemon, CLI, config, Python, and serial-related flows by avoiding panics when shared state is unexpectedly poisoned or serialization/parsing fails.
  • Some error messages are now more explicit, making failures easier to understand.

[coderabbitai]

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: 2bd06ee6-c93f-4a18-bdfe-92e4c8812f7e

📥 Commits

Reviewing files that changed from the base of the PR and between 861e8bf and 7d25a88.

📒 Files selected for processing (31)

• Cargo.toml
• ci/hooks/crate_guard.py
• crates/fbuild-build/src/compiler.rs
• crates/fbuild-build/src/esp32/orchestrator/build.rs
• crates/fbuild-build/src/pipeline/compile.rs
• crates/fbuild-build/src/pipeline/sequential.rs
• crates/fbuild-cli/src/cli/bloat_lookup.rs
• crates/fbuild-cli/src/cli/clang_tools.rs
• crates/fbuild-cli/src/lib_select.rs
• crates/fbuild-cli/src/mcp/server.rs
• crates/fbuild-config/src/bin/enrich_boards.rs
• crates/fbuild-config/src/ini_parser/parser.rs
• crates/fbuild-daemon/src/handlers/operations/build.rs
• crates/fbuild-daemon/src/handlers/operations/deploy.rs
• crates/fbuild-daemon/src/handlers/websockets.rs
• crates/fbuild-daemon/src/main.rs
• crates/fbuild-deploy/src/esp32/image.rs
• crates/fbuild-header-scan/src/scanner.rs
• crates/fbuild-paths/src/lib.rs
• crates/fbuild-python/src/async_serial_monitor.rs
• crates/fbuild-python/src/json_rpc.rs
• crates/fbuild-python/src/serial_monitor.rs
• crates/fbuild-serial/src/crash_decoder.rs
• dylints/README.md
• dylints/ban_std_sync_mutex_in_async/src/lib.rs
• dylints/ban_unwrap_in_daemon_handlers/README.md
• dylints/ban_unwrap_in_production/Cargo.toml
• dylints/ban_unwrap_in_production/README.md
• dylints/ban_unwrap_in_production/rust-toolchain.toml
• dylints/ban_unwrap_in_production/src/allowlist.txt
• dylints/ban_unwrap_in_production/src/lib.rs

---

📝 Walkthrough

Walkthrough

The ban_unwrap_in_daemon_handlers custom Dylint is renamed to ban_unwrap_in_production and its scope is widened to cover both crates/fbuild-daemon/src/ and crates/fbuild-cli/src/cli/. Test-file exemption logic is tightened to filename-pattern matching. All flagged unwrap call sites across the workspace are remediated with expect(...) or poison-tolerant unwrap_or_else.

Changes

ban_unwrap_in_production lint expansion and call-site remediation

Layer / File(s)	Summary
Dylint rename, scope expansion, and test-file exemption logic dylints/ban_unwrap_in_production/Cargo.toml, dylints/ban_unwrap_in_production/README.md, dylints/ban_unwrap_in_production/src/lib.rs, dylints/README.md, dylints/ban_std_sync_mutex_in_async/src/lib.rs	Renames crate to ban_unwrap_in_production, expands IN_SCOPE_PREFIXES to daemon + CLI/cli, rewrites is_test_file with explicit filename-pattern rules, updates lint message and unit tests, and updates the catalog README and mirror-lint comment.
Workspace config and CI guard updates Cargo.toml, ci/hooks/crate_guard.py	Updates the workspace exclude list and APPROVED_CRATE_DIRS allowlist to reference ban_unwrap_in_production.
Daemon handler unwrap fixes crates/fbuild-daemon/src/handlers/operations/build.rs, crates/fbuild-daemon/src/handlers/operations/deploy.rs, crates/fbuild-daemon/src/handlers/websockets.rs, crates/fbuild-daemon/src/main.rs	Replaces unwrap() with expect(...) in build/websocket/CORS handlers; changes AVR deploy config retrieval to ? error propagation.
CLI unwrap fixes crates/fbuild-cli/src/cli/bloat_lookup.rs, crates/fbuild-cli/src/cli/clang_tools.rs, crates/fbuild-cli/src/lib_select.rs, crates/fbuild-cli/src/mcp/server.rs	Replaces unwrap() with expect(...) for semaphore acquisition, query string derivation, JSON serialization, and MCP request ID extraction.
Build pipeline poison-tolerant mutex fixes crates/fbuild-build/src/compiler.rs, crates/fbuild-build/src/esp32/orchestrator/build.rs, crates/fbuild-build/src/pipeline/compile.rs, crates/fbuild-build/src/pipeline/sequential.rs	Replaces lock().unwrap() / into_inner().unwrap() with `unwrap_or_else(
Python serial monitor poison-tolerant mutex and expect fixes crates/fbuild-python/src/serial_monitor.rs, crates/fbuild-python/src/async_serial_monitor.rs, crates/fbuild-python/src/json_rpc.rs	Replaces unwrap() with `unwrap_or_else(
Remaining crate unwrap-to-expect fixes crates/fbuild-config/src/bin/enrich_boards.rs, crates/fbuild-config/src/ini_parser/parser.rs, crates/fbuild-deploy/src/esp32/image.rs, crates/fbuild-header-scan/src/scanner.rs, crates/fbuild-paths/src/lib.rs, crates/fbuild-serial/src/crash_decoder.rs	Replaces bare unwrap() with descriptive expect(...) calls across board enrichment, INI parser, ESP32 image parsing, header scanner, path utilities, and crash decoder regex initialization.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• FastLED/fbuild#849: Modifies the same ci/hooks/crate_guard.pyAPPROVED_CRATE_DIRS allowlist for dylints/* directories.

Poem

🐇 Hoppity hop through the code I go,
No more unwrap() lurking below!
Each panic now has a message clear,
And poisoned mutexes hold no fear.
From daemon to CLI, production's tight—
The lint shall guard us through the night! ✨

✨ 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/unwrap-prod-sweep-844

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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