Conversation
ExecCondition skip is terminal for the session, so a race with bluez at login (or after restarting bluetooth) leaves no pairing agent. Wait in ExecStartPre and order After=bluetooth.service so Restart=on-failure can recover. Closes omacom#13247
Automated AI review
Verified: the core change fixes the reported failure. A failed
Verified: the Moving the wait into the start job has two side effects that the PR description does not mention. The wait holds
|
|
Thanks for the thorough review — agreed on the side effects. Pushed follow-ups that:
Core fix vs |
Automated AI review
Outcome: The PR's
|
| Unit | Exit | Output |
|---|---|---|
| 3549e46 unit, unchanged | 0 | ok - bt-agent waits for bluetooth.service without holding the session target, ok - bt-agent still restarts after runtime failures, then the rest of the file passes |
| base unit (7b336b1) | 1 | not ok - bt-agent ExecCondition skip is terminal across a bluez race |
| earlier PR unit (ec28d53) | 1 | not ok - bt-agent ExecStartPre wait would hold graphical-session.target |
Restart=on-failure line deleted |
1 | not ok - bt-agent lost Restart=on-failure |
graphical-session.target dropped from After= |
1 | not ok - bt-agent must not hold graphical-session.target open |
Wants=bluetooth.service added |
1 | not ok - bt-agent must not depend on inert system bluetooth.service from the user manager |
ExecStartPre=/usr/bin/true added |
1 | not ok - bt-agent ExecStartPre wait would hold graphical-session.target |
ExecStart=/usr/bin/bt-agent -c NoInputNoOutput (no wait) |
1 | not ok - bt-agent must wait for bluetooth.service inside ExecStart, then exec bt-agent |
Safety comment lost pairable: true
Line 24 now reads # is only when the user .... Impact: the comment no longer states the condition it gives for auto-accept being safe.
Suggested change: restore `pairable: true`.
Reproducer
Reproducer: the auto-accept safety comment lost its key phrase
At 3549e46 (the change came in 0ab5f5d):
$ grep -n 'is only' default/systemd/user/bt-agent.service
24:# is only when the user explicitly opens the omarchyAt the base (7b336b1):
$ grep -n 'is only' default/systemd/user/bt-agent.service
15:# is only `pairable: true` when the user explicitly opens the omarchyAt the earlier PR head (ec28d53):
$ grep -n 'is only' default/systemd/user/bt-agent.service
17:# is only `pairable: true` when the user explicitly opens the omarchyThe diff of 0ab5f5d shows the change on that line:
-# is only `pairable: true` when the user explicitly opens the omarchy
+# is only when the user explicitly opens the omarchyThe backtick-quoted pairable: true was dropped, which leaves a double space and a sentence that no longer says when auto-accepting is safe.
Details
Verified (sandbox runs of the unit's ExecStart script with stubbed systemctl and bt-agent, plus systemd, bluez and bluez-tools source):
- The wait runs in
ExecStart, so theType=simplestart job completes at fork.After=graphical-session.targetremoves the target's implicit ordering after the unit without creating a cycle, the same pattern asomarchy-migrate-notify.service. - bluez inactive, then active: the shell polls every 2 s, then
execsbt-agent -c NoInputNoOutputin the same PID, and bt-agent's exit status passes through to systemd. - bluez off: one shell calls
systemctl is-activeevery 2 s, with no restarts and no journal lines. SIGTERM ends the waiting shell at once.enable --nowin first-run no longer waits for bluez. - A mid-session
systemctl restart bluetooth.servicestill recovers: afterRelease, bt-agent'sUnregisterAgentcall fails and it exits 1, soRestart=on-failurefires (matching the journal in bt-agent.service is permanently skipped when its ExecCondition races bluetooth.service, leaving the session with no Bluetooth pairing agent #13247). - The test's
ExecConditionandExecStartPrechecks fail with their intended messages against the base unit and the ec28d53 unit, respectively.
PR description: it still describes After=/Wants=bluetooth.service, a bounded ExecStartPre wait and passing bt-agent assertions, which no longer match the head.
Related open PRs: #9131 and #11937 still conflict with this PR in both files. #13620 (another fix for #13247) also conflicts in both files, and #8929, #12897 and #9965 edit the same ExecStart block.
Review information
Test scope: Source and sandbox tests only at 3549e46; no real systemd user session or bluez.
AI process: Opus 5.5 Medium coordination and synthesis, Opus 5.5 Xhigh technical review and final fact check, GPT 6 Sol Xhigh search for related issues, Opus 5.5 Medium editorial check.
Opt out: To stop receiving these reviews, reply to this comment saying so.
Anchor systemd-test greps to directive lines so combined After= and comments cannot false-fail/false-pass. Restore pairable: true in the safety comment.
|
Thanks — fixed on head:
|
|
Reviewed at ae62e23 and reproduced on a disposable Omarchy VM (systemd 261.2, bluez 5.87, bluez-tools 0.2.0). It fixes the bug, and nothing was found that needs changing. Reproduction. With Tests. On the same VM, Second opinion. Codex Medium reviewed the diff and found no defect that changes behaviour, which agrees with my review (its independence isn't guaranteed). It raised two smaller points that I verified. Neither blocks the PR, and I've left both alone. First, the new assertions match directive text, so a broken loop ( Competing fixes. #9131, #11937 and #13620 change the same unit for the same bug. Codex was asked to compare all four without being told my answer, and it chose this one, as I did. #9131 (bounded I've linked #8962 as well, since it's the same login race. This now waits on the maintainer. |
|
Thanks for reproducing this on a real Omarchy VM and comparing the competing fixes. Glad the three failure cases (start while bluez is down, mid-session bluez restart, first-run Happy to rebase or adjust if anything else comes up. Waiting on maintainer review. |
Problem
bt-agent.serviceusedExecCondition=systemctl is-active bluetooth.service. When that condition fails, systemd skips the unit. A skip is terminal for that activation —Restart=on-failuredoes not try again — so a race with bluez at login (or aftersystemctl restart bluetooth) leaves the session with no pairing agent. The radio still scans; pairing never completes.Reported in #13247 with journal evidence (
Skipped due to 'exec-condition'until bluetoothd is already up).Change
bluetooth.serviceinsideExecStart, thenexec bt-agent, so a late bluez is no longer a terminal skip and the Type=simple start job does not holdgraphical-session.target.After=graphical-session.target(same pattern as migrate-notify) so the unit cannot pin the session target open.After=/Wants=on systembluetooth.service(those deps are inert in the user manager).ConditionPathIsDirectory=/sys/class/bluetoothso machines with no Bluetooth class still skip cleanly.systemd-test.shwith directive-anchored assertions for the wait + ordering.Test
ROOT=... bash test/shell.d/systemd-test.sh— bt-agent assertions pass.Closes #13247
Fixes #8962