Skip to content

feat(medium): Fix Bluetooth Auto-Connect Race Condition and Overlapping Connections#9551

Open
arii wants to merge 21 commits intoleaderfrom
fix-bluetooth-race-condition-17974409934683189531
Open

feat(medium): Fix Bluetooth Auto-Connect Race Condition and Overlapping Connections#9551
arii wants to merge 21 commits intoleaderfrom
fix-bluetooth-race-condition-17974409934683189531

Conversation

@arii
Copy link
Copy Markdown
Owner

@arii arii commented Mar 16, 2026

Description

This PR addresses a race condition in the Bluetooth connection flow by implementing several guards and improvements to ensure stable and accessible connections.

Fixes #9303

Changes Made

  • Guarded Auto-Connect: The useAutoConnect logic now checks for a saved device ID before attempting a silent connection, preventing unnecessary error logs.
  • Connection Handshake Guard: A ref-based lock (isConnecting) was added to useBluetoothHRM to ensure that manual and auto-connect attempts do not overlap, which previously caused AbortController triggers and potential radio blocking.
  • Accessibility Improvements: ARIA live regions were added to the connection status components to ensure state changes (like "Connecting..." or "Failed") are announced to assistive technologies.
  • Graceful Failures: Silent connection attempts now exit gracefully instead of throwing errors when no device is found.

Testing

Verified with unit tests and manual frontend verification.

Change Type: 🐛 Bug fix (non-breaking change fixing an issue)

Related Issues

Closes #9303

Original PR Body

This PR addresses a race condition in the Bluetooth connection flow. Specifically:

  1. Guarded Auto-Connect: The useAutoConnect logic now checks for a saved device ID before attempting a silent connection, preventing unnecessary error logs.
  2. Connection Handshake Guard: A ref-based lock (isConnecting) was added to useBluetoothHRM to ensure that manual and auto-connect attempts do not overlap, which previously caused AbortController triggers and potential radio blocking.
  3. Accessibility Improvements: ARIA live regions were added to the connection status components to ensure state changes (like "Connecting..." or "Failed") are announced to assistive technologies.
  4. Graceful Failures: Silent connection attempts now exit gracefully instead of throwing errors when no device is found.

Verified with unit tests and manual frontend verification.

Fixes #9303


PR created automatically by Jules for task 17974409934683189531 started by @arii

…nnection attempts

- Added a check for `hrm_device_id` cookie before triggering auto-connect in `ConnectPage`.
- Implemented an `isConnecting` ref guard in `useBluetoothHRM` hook to prevent concurrent connection handshakes.
- Updated `connectAndStream` to return gracefully on silent connection failures.
- Added `role="status"` and `aria-live="polite"` to Bluetooth status elements for improved accessibility.
- Updated unit tests to reflect the new guarded connection behavior.

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@github-actions
Copy link
Copy Markdown
Contributor

👋 Welcome to HRM!

Thanks for your contribution. This repository uses Gemini AI for automated triage, code review, and generation.

🤖 Gemini Manual Trigger Quick Reference

Command Action
@gemini-bot Run AI Code Review (PR only)
@gemini-triage Run Issue Triage
@gemini-coder <task> Generate Code
@create-review-issues Create issues from review (PR only)
@gemini-help Show this help message
@pr-squash Squash PR commits (PR only)
@conflict-resolve Resolve merge conflicts (PR only)

For more details and GitHub CLI examples, see the Manual Trigger Guide.

@github-actions github-actions bot changed the title Fix Bluetooth Auto-Connect Race Condition and Overlapping Connections feat(medium): Fix Bluetooth Auto-Connect Race Condition and Overlapping Connections Mar 16, 2026
@github-actions
Copy link
Copy Markdown
Contributor

ℹ️ Gemini Review Disabled

Automatic review is currently disabled via GEMINI_ENABLE_PR_REVIEW. To run review manually:

Via Comment:

@gemini-bot

Via GitHub Actions:

  1. Go to Actions → Reusable Gemini Review
  2. Click "Run workflow"
  3. Enter PR number: 9551

Via GitHub CLI:

gh workflow run reusable-gemini-review.yml -f pr_number=9551

🤖 Manual Trigger Quick Reference

Command Action
@gemini-bot Run AI Code Review (PR only)
@gemini-triage Run Issue Triage
@gemini-coder <task> Generate Code
@create-review-issues Create issues from review (PR only)
@gemini-help Show this help message

Full reference: Manual Trigger Guide

To re-enable: Set repository variable GEMINI_ENABLE_PR_REVIEW=true

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Manual Trigger Quick Reference

Command Action
@gemini-bot Run AI Code Review (PR only)
@gemini-triage Run Issue Triage
@gemini-coder <task> Generate Code
@create-review-issues Create issues from review (PR only)
@gemini-help Show this help message

Manual Trigger Guide

@arii arii added the not reviewed The review could not be performed or completed successfully. label Mar 16, 2026
- Added a check for `hrm_device_id` cookie before triggering auto-connect in `ConnectPage`.
- Implemented an `isConnecting` ref guard in `useBluetoothHRM` hook to prevent concurrent connection handshakes.
- Updated `connectAndStream` to return gracefully on silent connection failures.
- Added `role="status"` and `aria-live="polite"` to Bluetooth status elements for improved accessibility.
- Updated unit tests to reflect the new guarded connection behavior.
- Fixed Prettier formatting in `hooks/useBluetoothHRM.ts`.

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Manual Trigger Quick Reference

Command Action
@gemini-bot Run AI Code Review (PR only)
@gemini-triage Run Issue Triage
@gemini-coder <task> Generate Code
@create-review-issues Create issues from review (PR only)
@gemini-help Show this help message

Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 16, 2026
@arii
Copy link
Copy Markdown
Owner Author

arii commented Mar 16, 2026

@Gemini-bot

@arii
Copy link
Copy Markdown
Owner Author

arii commented Mar 16, 2026

Reviewed commit: 67f54b1a2ade7bf07decdad217c8d19b851fae2d

This PR effectively addresses the Bluetooth auto-connect race condition and overlapping connections, significantly improving the stability of the connection process. The addition of role="status" and aria-live="polite" to the connection status elements is a valuable accessibility improvement, aligning with the project's DESIGN_GUIDELINES.md.

Key Changes & Improvements:

  • Race Condition Fix: The logic in app/client/connect/page.tsx now correctly checks for a savedDeviceId before attempting an auto-connect, preventing unnecessary connection attempts.
  • Overlapping Connection Prevention: The isConnecting.current flag management in hooks/useBluetoothHRM.ts has been refactored to centralize the check in connectAndStream, ensuring that only one connection attempt can be active at a time.
  • Accessibility: Enhanced screen reader support for connection status updates in app/client/connect/ConnectView.tsx.
  • Test Update: The unit test tests/unit/hooks/useBluetoothHRM.race.test.ts was correctly updated to reflect the new behavior of preventing concurrent connections.

Minor Issue Identified:

While the isConnecting.current flag is now correctly reset in a finally block, its initial assignment to true at hooks/useBluetoothHRM.ts:732 is placed inside the try block. This means if an error occurs before that line (e.g., during Bluetooth.requestDevice at line 726), the isConnecting.current flag will not have been set to true for the entire duration of the function's critical path. Although the initial check at line 720 prevents immediate re-entry, setting the flag earlier would ensure it guards the entire connection process from its very beginning, making the race condition prevention more robust.

Recommendation: Move isConnecting.current = true to immediately after the initial if (isConnecting.current) check at hooks/useBluetoothHRM.ts:724.

Overall, this is a well-executed PR that delivers important fixes and improvements.

Reviewed at commit: 67f54b1a2ade7bf07decdad217c8d19b851fae2d


🤖 Gemini Manual Trigger Quick Reference

Command Action
@gemini-bot Run AI Code Review (PR only)
@gemini-triage Run Issue Triage
@gemini-coder <task> Generate Code
@create-review-issues Create issues from review (PR only)
@gemini-help Show this help message

Manual Trigger Guide

@arii arii added bug Something isn't working accessibility refactor approved and removed not reviewed The review could not be performed or completed successfully. labels Mar 16, 2026
… feedback

- Moved `isConnecting.current = true` before the `try` block in `connectAndStream` to ensure the entire connection path is guarded.
- Maintained all previous fixes for auto-connect race conditions and accessibility.
- Verified with unit tests and linting.

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Manual Trigger Quick Reference

Command Action
@gemini-bot Run AI Code Review (PR only)
@gemini-triage Run Issue Triage
@gemini-coder <task> Generate Code
@create-review-issues Create issues from review (PR only)
@gemini-help Show this help message

Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed approved labels Mar 16, 2026
- Added a check for `hrm_device_id` cookie before triggering auto-connect in `ConnectPage`.
- Implemented an `isConnecting` ref guard in `useBluetoothHRM` hook to prevent concurrent connection handshakes.
- Moved `isConnecting.current = true` before the `try` block in `connectAndStream` to ensure entire path is guarded.
- Fixed a scoping issue with `silent` variable in `connectAndStream` that caused CI type check failure.
- Added `role="status"` and `aria-live="polite"` to Bluetooth status elements for improved accessibility.
- Updated unit tests to reflect the new guarded connection behavior.
- Verified with unit tests, linting, and type checking.

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Manual Trigger Quick Reference

Command Action
@gemini-bot Run AI Code Review (PR only)
@gemini-triage Run Issue Triage
@gemini-coder <task> Generate Code
@create-review-issues Create issues from review (PR only)
@gemini-help Show this help message

Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 16, 2026
… feedback

- Added a check for `hrm_device_id` cookie before triggering auto-connect in `ConnectPage`.
- Implemented an `isConnecting` ref guard in `useBluetoothHRM` hook to prevent concurrent connection handshakes.
- Moved `isConnecting.current = true` before the `try` block in `connectAndStream` to ensure entire path is guarded, as recommended in code review.
- Fixed a scoping issue with `silent` variable in `connectAndStream` that caused CI type check failure.
- Added `role="status"` and `aria-live="polite"` to Bluetooth status elements for improved accessibility.
- Updated unit tests to reflect the new guarded connection behavior.
- Verified with unit tests, linting, and type checking.
- Analyzed and confirmed VRT failures are unrelated to these changes.

Co-authored-by: arii <342438+arii@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Manual Trigger Quick Reference

Command Action
@gemini-bot Run AI Code Review (PR only)
@gemini-triage Run Issue Triage
@gemini-coder <task> Generate Code
@create-review-issues Create issues from review (PR only)
@gemini-help Show this help message

Manual Trigger Guide

@arii arii removed the not reviewed The review could not be performed or completed successfully. label Mar 16, 2026
@arii arii added the enhancement New feature or request label Mar 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed approved labels Mar 23, 2026
@arii
Copy link
Copy Markdown
Owner Author

arii commented Mar 23, 2026

🤖 AI Technical Audit

Architectural Impact

The introduction of connectionLock and the extraction of bluetoothStorage utilities significantly improves the robustness of the Bluetooth state machine. By moving from a reactive to a defensive connection posture, we reduce the risk of 'GATT Server' collisions which are notoriously difficult to debug in browser environments. However, the PR introduces several regressions in test stability and some over-engineered logic in the constants file.

File-by-File Analysis

app/client/connect/page.tsx

  • Problem: The useEffect for auto-connect has become overly complex with the manual extraction of savedDeviceId. This creates a secondary source of truth inside the component that might lag behind the internal state of useBluetoothHRM.
  • Implementation Sample:
// Recommendation: Keep the logic inside the hook. 
// The component should just call autoConnect(); the hook should decide if it has the ID.
const autoConnect = useCallback(async () => {
  const id = getSavedDeviceId();
  if (!id) return;
  // ... rest of logic
}, []);

hooks/useBluetoothHRM.ts

  • Problem: You renamed isConnecting to connectionLock but removed the .current = false reset from the connectToGatt finally block, moving it to connectAndStream. This creates a leak where if an internal call to connectToGatt fails, the lock might remain trapped if not called through the public wrapper.
  • Problem: isTimeoutDisconnect.current = false was deleted in multiple places. If this ref was used to distinguish between manual and system-induced disconnects elsewhere, this will break reconnection logic.

constants/bluetooth-reconnection.ts

  • Problem: Over-engineering the HEARTBEAT_INTERVAL_MS constant. Exporting environment-specific logic in a constants file makes it harder to tree-shake and complicates unit tests.

tests/playwright/vrt-dashboard.spec.ts

  • Problem: High code churn. The PR refactors the entire test suite setup while only intending to fix a race condition. This obscures the actual changes needed for the bug fix.

ANTI-AI-SLOP DIRECTIVES

  1. OVERLY VERBOSE COMMENTS: The comment // Abort silent connection if no device ID is found, to prevent looping. was removed, which is good. However, // This is reset at the end of the function... was also removed but replaced with no explanation of the new locking mechanism.
  2. OVER-ENGINEERING: HEARTBEAT_INTERVAL_MS logic using process.env.NODE_ENV inside the constants file is unnecessary. Use a mock or a configuration provider.
  3. DUPLICATE HOOKS/TYPES: The bluetoothStorage.ts is a good addition, but ensure js-cookie isn't already wrapped in a useStorage hook elsewhere in the project.
  4. CODE RATIO: 12 lines in vrt-dashboard.spec.ts (the new beforeEach block) can be simplified by using a shared setup utility instead of re-implementing the WebSocket wait logic in every file.
  5. STALE FEATURES: The isTimeoutDisconnect ref usage was removed from the code but the definition likely still exists in the hook's top-level useRef calls. Remove the definition to avoid stale memory usage.

Review automatically published via RepoAuditor.

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 23, 2026
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: arii <342438+arii@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 26, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 26, 2026
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: arii <342438+arii@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

@arii arii added not reviewed The review could not be performed or completed successfully. and removed not reviewed The review could not be performed or completed successfully. labels Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Race condition in Bluetooth Auto-Connect and overlapping connection attempts

1 participant