fix(install): reload udev rules before re-probing drivers (#355)

[BANANASJIM]

Summary

Confirmed install-migration race surfaced by adversarial review of #365 and reproduced on a real Flydigi Vader 5.

probeAndReprobeDrivers writes driverless interfaces to /sys/bus/usb/drivers_probe, which makes the kernel re-bind drivers and emit bind uevents. udevd evaluates those uevents against its currently loaded ruleset. The re-probe ran inside installUdevRules — before the later udevadm control --reload-rules — so a stale block usbhid rule from a previous (#357-style) install was still loaded and re-unbound usbhid on the bind event, leaving the device with no hidraw node ("no device") after an update until replug/reboot.

Fix

• Move live driver-state mutation (re-probe + proactive unbind) out of installUdevRules into a new applyDriverState; installUdevRules is now write-only.
• Call applyDriverState only after udevadm control --reload-rules, and before starting the service (split runSystemctlPhase → runSystemctlUnits; reload+trigger now run once in phase.zig for both the enable and non-enable paths).

Final order: write rule files → sweep stale files → reload udevd ruleset → mutate driver state → start service.

Test plan

• Real-hardware repro (Flydigi Vader 5): seeded a stale block usbhid rule loaded in udevd + sentinel, then ran both orders. Old order (re-probe before reload) → hidraw=0 (pad invisible). New order (reload before re-probe) → hidraw=6 (pad discoverable). Cleanup restored stock state.
• zig build test (full suite) green in the canonical Docker image.
• zig build test-tsan green in Docker.

Summary by CodeRabbit

• Refactor
  • Reorganized the installation process flow to adjust when udev rules are reloaded and driver state is applied relative to systemctl service initialization.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6856b970-edb3-4e96-93ad-51a6099a88db

📥 Commits

Reviewing files that changed from the base of the PR and between 4304561 and fb3813b.

📒 Files selected for processing (3)

• src/cli/install/phase.zig
• src/cli/install/services.zig
• src/cli/install/udev.zig

---

📝 Walkthrough

Walkthrough

This PR refactors the install phase's device driver and systemctl management by extracting driver state application into a standalone function and reordering the execution sequence. Udev reload/trigger now happens in the phase orchestrator, followed by the new applyDriverState() call, with systemctl units started last.

Changes

Install Phase Udev and Systemctl Reordering

Layer / File(s)	Summary
Driver state application extraction src/cli/install/udev.zig	Introduced exported applyDriverState() that encapsulates driver reprobe and conditional proactive unbinding. The function early-returns in staging mode or when not root, then triggers device reprobe and conditionally unbinds via shouldProactiveUnbind(plan) guard.
Systemctl function refactoring src/cli/install/services.zig	Renamed runSystemctlPhase to runSystemctlUnits and removed embedded udev reload/trigger logic. The function now assumes udev rules and driver state are already handled by the caller and proceeds directly to systemctl daemon-reload, enable, and start operations.
Install phase execution reordering src/cli/install/phase.zig	Reorganized the install phase to reload udev rules (when enabling systemctl or not in staging mode), apply driver state via applyDriverState(), then run systemctl units via runSystemctlUnits(). Separates concerns by moving udev/driver handling before systemctl startup.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• BANANASJIM/padctl#256: Defines the shouldProactiveUnbind(plan) sentinel that gates the proactive unbind behavior in the new applyDriverState().
• BANANASJIM/padctl#365: Implements the underlying probeAndReprobeDrivers() and unbinding logic that applyDriverState() calls.
• BANANASJIM/padctl#351: Defines the LifecycleScope and InstallPlan.isStaging() logic that determines staging/system behavior checked in the new applyDriverState().

Poem

🐰 The drivers dance in rhythm true,
Udev rules reload, then reprobe anew,
No more tangled systemctl threads,
Clean separation feeds the reds! 🐾

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(install): reload udev rules before re-probing drivers (#355)' directly and accurately describes the main change: ensuring udev rules are reloaded before driver re-probing occurs, which is the core fix addressing the ordering issue that was causing stale rules to interfere with device binding.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/355-reload-before-reprobe

---

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 and usage tips.}