Skip to content

Avoid connecting after Bluetooth pair failures - #383

Open
Brams-s wants to merge 1 commit into
omacom:quattrofrom
Brams-s:fix/bluetooth-pair-failure
Open

Brams-s wants to merge 1 commit into
omacom:quattrofrom
Brams-s:fix/bluetooth-pair-failure

Conversation

@Brams-s

@Brams-s Brams-s commented Sep 9, 2026

Copy link
Copy Markdown

Summary

  • Only trust and connect a Bluetooth device after bluetoothctl pair succeeds.
  • Cover successful pairing and ensure a failed pairing does not leave an unpaired device trusted or connected.

Testing

  • bash test/shell.d/bluetooth-test.sh
  • ./test/cli
  • bin/omarchy commands --check
  • Full shell suite: 255/256 test files pass; the unrelated test/shell.d/launch-about-test.sh fails its existing roomy-window animation expectation.

@Brams-s
Brams-s force-pushed the fix/bluetooth-pair-failure branch from 242db89 to e691ebd Compare September 10, 2026 18:53
@Brams-s
Brams-s force-pushed the fix/bluetooth-pair-failure branch from e691ebd to f6e1c66 Compare September 10, 2026 20:01
@duketopceo

Copy link
Copy Markdown

Verified on MacBookPro18,1 (M1 Pro, aarch64, BCM4387 / hci_bcm4377), Omarchy 4.0.3-1:

  • bluetooth-test.sh: all pass, including the 6 new pair-failure cases (pair attempted, no trust, no connect on failure).
  • Live: omarchy-bluetooth-device pair 00:00:00:00:00:00 on the real adapter exits cleanly in ~14ms — pair returns "Device not available" (exit 1) and the new if correctly skips trust/connect. Under the old || true chain both would still have fired against a device with no pairing record, which is exactly the half-paired "connected but no A2DP sink" state this fixes.

LGTM from a test standpoint.

@malik-na

Copy link
Copy Markdown
Member

Validated head f6e1c66af3e77486cdb38614b5707b6217d4e6e0 against freshly fetched quattro 3246b6b4d84fd5339b4c5e8576a23d5199f7087a (including #387). The clean local merge produced tree 6d28d1a3bbeb905467688b89e81e66e9b8d8912b.

Verified pass

  • Expanded device matrix: 23/23; shared Bluetooth suite: 54 assertions; independent power-order check passes. The 18 unaffected matrix cases match baseline traces and exit statuses.
  • Five baseline negative controls reproduce trust/connect after unsuccessful pairing; the candidate correctly skips those follow-ups. This includes the command's actual, unmodified 20-second timeout.
  • The installed BlueZ 5.87-2 bluetoothctl binary passes 17 private-D-Bus fixture cases covering success, authentication errors, unavailable devices/adapters, and real timeout. This supports the exit-status contract used by the new condition; it is not hardware authentication evidence.
  • Broad comparison found no candidate-specific regression: both refs pass 250 non-graphical shell files, 116 CLI assertions, 468 metadata checks, 472 command parser checks, and 11 legacy suites across initial and targeted fixture reruns.
  • fix(bluetooth): power off adapters via BlueZ before rfkill block to prevent PCIe driver lockup #380 shares bluetooth-test.sh. A clean combined merge at its inspected head 40c0ac2196c6862fee7b459f244e3ea33e5b4df4 retains both changes' assertions and passes the focused checks. This establishes shared shell/test compatibility, not fix(bluetooth): power off adapters via BlueZ before rfkill block to prevent PCIe driver lockup #380's hardware safety or general merge readiness.
  • No migration is needed for this runtime-only gating change. Existing installs receive it with the updated command; fresh installs use the same command. It does not repair existing trusted-but-unpaired records.

Remaining coverage gaps

  • Physical iPhone pairing was attempted on the host's existing Bluetooth stack. Two exact-candidate attempts skipped trust/connect after failure. During overlapping manual attempts, the phone showed a code but pairing did not complete; the Mac reported Paired=no, Bonded=no, Trusted=yes, Connected=no. Neither candidate trace contains a trust call, so that trust state is not attributed to this PR.
  • A separate direct interactive bluetoothctl attempt, outside the wrapper, ended in AuthenticationTimeout before matching-code confirmation completed on both sides. Earlier service logs show a discovery abort but do not establish the root cause. This is unresolved physical coverage, not a reproduced PR regression. No successful bond, A2DP/audio, reconnect, or reboot-persistence result is claimed. Further phone testing is paused; temporary agents and discovery were stopped.
  • Twelve graphical suites self-skip; root/loop and several hardware/network/ISO cases remain untested. Four suites were excluded for isolation/scope constraints. ShellCheck is unavailable locally. There is no full-suite-green claim: targeted fixture reruns resolved terminal/setup harness failures, but the original aggregate was not rerun.
  • The four recorded hosted checks are successful, including ARM installation, but date to September 10 and do not test this new local merged tree. GitHub currently reports MERGEABLE / BLOCKED with REVIEW_REQUIRED.

Reproduced bug, pre-existing on both refs: the network-QR command exits on a failed route lookup before its intended fallback. A canned-route continuation passes the remaining fixture cases; it does not fix that existing failure and is not a #383 regression.

No introduced defect was reproduced in #383. The bounded code/contract checks pass, but I am not giving an unconditional merge recommendation from these results. Required review and the explicitly unresolved physical coverage remain visible.

@wesleygrimes wesleygrimes added the cherry-pick-later Port to omacom/omarchy after the overlay merges; match mainline. label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-pick-later Port to omacom/omarchy after the overlay merges; match mainline.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants