Skip to content

fix: restore green main - clippy needless borrow + unix pid-0 liveness (#1213 follow-up) - #1229

Merged
zackees merged 1 commit into
mainfrom
fix/main-clippy-needless-borrow
Aug 1, 2026
Merged

fix: restore green main - clippy needless borrow + unix pid-0 liveness (#1213 follow-up)#1229
zackees merged 1 commit into
mainfrom
fix/main-clippy-needless-borrow

Conversation

@zackees

@zackeeszackees commented Aug 1, 2026

Copy link
Copy Markdown
Member

Closes#1213

Summary

PR #1227 was admin-merged with the Check jobs red, breaking main two ways. This PR fixes both:

  1. Clippy hard error (needless_borrows_for_generic_args, daemon_cmd.rs:26): pass format!(...) by value instead of &format!(...)output::result takes impl Display, and -D warnings turns the lint into an error. This is what turned Check Ubuntu/Windows red at the merge commit (green on the prior push, fix(ldf): honor lib_deps on Teensy/STM32 and warn on inert lib_ldf_mode (#1214) #1226).
  2. Unix PID-liveness bug: pid_is_alive used kill(pid, 0) == 0, but kill(0, 0) probes the caller's own process group and succeeds — so PID 0 read as "alive" on macOS/Linux. That failed fix(packages,daemon): reclaim dead-owner install locks; stop leaving stale endpoint records (#1213) #1227's two install-lock staleness tests on Check macOS (Ubuntu masked behind the clippy error; Windows unaffected, which is why it slipped through local dev). It is also a real production hole: a corrupt owner.txt recording pid=0 would never be reclaimed — the exact Orphaned mkdir install-locks deadlock after daemon crash; stale daemon endpoint survives 'daemon stop'; esptool provisioning failure kills daemon #1213 deadlock class. Fixed with an explicit pid-0 guard + cross-platform regression test.

With this follow-up, all three parts of #1213 are fully landed (dead-owner install-lock reclaim + stale endpoint clearing shipped in #1227).

Validation

  • soldr cargo clippy -p fbuild-cli --all-targets -- -D warnings — clean (previously reproduced the exact CI error).
  • soldr cargo test -p fbuild-core process_identity — 7 passed (incl. new pid_zero_is_never_alive).
  • soldr cargo test -p fbuild-packages-fetch install_lock — 12 passed.
  • Root causes confirmed against CI logs: run 30720560199 (Check Ubuntu, main — clippy error) and job 91426904142 (Check macOS, this PR — the two install_lock test failures).

🤖 Generated with Claude Code

…w-up)
PR #1227 was merged with the Check jobs red: clippy denies
needless_borrows_for_generic_args on `output::result(&format!(...))`
in daemon_cmd.rs (output::result takes impl Display, so the borrow is
needless). The fix existed locally but was never committed. Pass the
String by value, restoring green Check Ubuntu/Windows/macOS on main.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1311a7ca-0328-42f4-b59e-e5a602a4bb7a

📥 Commits

Reviewing files that changed from the base of the PR and between 5a0bc88 and 15a0ee1.

📒 Files selected for processing (1)
  • crates/fbuild-cli/src/cli/daemon_cmd.rs

📝 Walkthrough

Walkthrough

The daemon command now passes the formatted stale-record message directly to output::result. The displayed output remains unchanged.

Changes

Daemon output formatting

Layer / File(s)Summary
Stale-record reporting
crates/fbuild-cli/src/cli/daemon_cmd.rs
run_daemon passes the format! result directly to output::result.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check nameStatusExplanationResolution
Linked Issues check⚠️ WarningThe change fixes Clippy but does not demonstrate the lock, endpoint, or esptool recovery requirements listed in issue #1213.Provide links and evidence for the separate implementations, or include the missing #1213 changes in this pull request.
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the needless-borrow fix that resolves the Clippy failure.
Out of Scope Changes check✅ PassedThe one-line change is directly related to the stated CLI follow-up and restores Clippy checks.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/main-clippy-needless-borrow

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.

@zackees
zackees merged commit fe9a672 into mainAug 1, 2026
91 of 93 checks passed
zackees added a commit that referenced this pull request Aug 1, 2026
kill(0, 0) probes the caller's own process group and succeeds, so the
unix liveness check reported pid 0 as alive. That failed the two #1227
install-lock staleness tests on macOS/Linux (Check macOS red on PR
#1229; Ubuntu masked behind the clippy error), and in production a
corrupt owner record holding pid=0 would never be reclaimed — the
exact #1213 deadlock class. Guard pid 0 explicitly and add a
cross-platform regression test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@zackeeszackees changed the title fix(cli): drop needless borrow that broke clippy on main (#1213 follow-up)fix: restore green main - clippy needless borrow + unix pid-0 liveness (#1213 follow-up)Aug 1, 2026
Sign up for freeto 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.

Orphaned mkdir install-locks deadlock after daemon crash; stale daemon endpoint survives 'daemon stop'; esptool provisioning failure kills daemon

1 participant

@zackees