Skip to content

fix(ios): force reconnect bypasses disconnected connection guard#6156

Merged
mdmohsin7 merged 3 commits into
mainfrom
fix/ios-force-reconnect-bypass
Mar 29, 2026
Merged

fix(ios): force reconnect bypasses disconnected connection guard#6156
mdmohsin7 merged 3 commits into
mainfrom
fix/ios-force-reconnect-bypass

Conversation

@mdmohsin7
Copy link
Copy Markdown
Member

@mdmohsin7 mdmohsin7 commented Mar 29, 2026

Summary

  • ensureConnection returned null for the same device when disconnected, even with force=true (user-initiated reconnect)
  • After the first connection + disconnect, the _connection object persists with the matching device ID but disconnected status
  • Line 237 (if (_connection?.device.id == deviceId) return null) blocks ALL subsequent reconnection attempts regardless of force — the force flag was never checked on that path
  • This made user-initiated reconnect taps silently do nothing on iOS (Android was unaffected because _connection gets fully cleaned up on disconnect)
  • Fix: add !force && so force=true calls fall through to _connectToDevice

Test plan

  • iOS: disconnect device manually, tap to reconnect — should reconnect
  • iOS: device goes out of range and back — native auto-reconnect still works (force=false path unchanged)
  • Android: no behavioral change expected

🤖 Generated with Claude Code

ensureConnection returned null for the same device when disconnected,
even with force=true. This meant user-initiated reconnect taps were
silently ignored on iOS where native auto-reconnect left a stale
_connection object with the correct device ID but disconnected status.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 29, 2026

Greptile Summary

This PR fixes a silent no-op on iOS when a user manually taps to reconnect a device that is in a stale-disconnected state. The root cause was that ensureConnection returned null for any existing _connection with the correct device ID, even when force=true; the one-line guard change (!force && ...) correctly lets user-initiated reconnects fall through to _connectToDevice, which tears down the stale transport and creates a fresh connection.

Changes:

  • ensureConnection: the disconnected-device guard now checks !force first, so force=true calls skip it and proceed to _connectToDevice — fixing the user-tap reconnect regression on iOS.
  • _connectToDevice + ensureConnection: six raw print() debugging statements were added alongside the fix. These are inconsistent with the rest of the file (which uses Logger.debug) and will appear in release builds without any log-level filtering. They should be converted to Logger.debug() or removed before merging.
  • The force=false path (native auto-reconnect) is completely unchanged.

Confidence Score: 4/5

Safe to merge after removing the debug print() statements; the logic fix itself is correct and well-scoped.

The one-line guard change is correct and the described iOS scenario is well-reasoned. Score is 4 rather than 5 because six raw print() calls were shipped alongside the fix — these bypass log filtering and will emit to stdout in production/release builds, which is a real (if minor) regression in logging hygiene that should be cleaned up before merging.

app/lib/services/devices.dart — the debug print() statements need to be converted to Logger.debug() or removed

Important Files Changed

Filename Overview
app/lib/services/devices.dart Core fix: adds !force && guard so user-initiated reconnects (force=true) bypass the stale-connection short-circuit and fall through to _connectToDevice. Six debug print() calls were also added but should be Logger.debug() before merging.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["ensureConnection(deviceId, force)"] --> B{Same device & connected?}
    B -- Yes --> C[return _connection]
    B -- No --> D{"!force && same device ID? (stale disconnected)"}
    D -- "true (force=false)" --> E[return null - native auto-reconnect handles it]
    D -- "false (force=true OR different device)" --> F{force?}
    F -- No --> G[return null]
    F -- Yes --> H["_connectToDevice(deviceId)"]
    H --> I{existing _connection?}
    I -- Yes, connected --> J[disconnect + dispose transport]
    I -- Yes, disconnected --> K[dispose transport only]
    J --> L[_connection = null]
    K --> L
    I -- No --> M[lookup device]
    L --> M
    M --> N[DeviceConnectionFactory.create]
    N --> O[connect with callback]
    O --> P[return _connection]
Loading

Comments Outside Diff (2)

  1. app/lib/services/devices.dart, line 125-134 (link)

    P1 Debug print statements left in production code

    Six raw print() calls were added in _connectToDevice and ensureConnection (lines 125, 128, 132, 134, 224, 226) but the rest of this file consistently uses Logger.debug() for diagnostic output. Raw print() calls bypass any log-level filtering, log capture, or DebugLogManager integration and will appear verbatim in release builds.

    These should either be removed or converted to Logger.debug():

  2. app/lib/services/devices.dart, line 224-226 (link)

    P1 print instead of Logger.debug in ensureConnection

    Same issue as in _connectToDevice above — these two print() calls in ensureConnection should be Logger.debug() to be consistent with the rest of the file and to respect release-build log suppression.

Reviews (1): Last reviewed commit: "chore: remove debug print statements" | Re-trigger Greptile

@mdmohsin7 mdmohsin7 merged commit 5aa4592 into main Mar 29, 2026
2 checks passed
@mdmohsin7 mdmohsin7 deleted the fix/ios-force-reconnect-bypass branch March 29, 2026 09:07
Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
…edHardware#6156)

## Summary

- `ensureConnection` returned `null` for the same device when
disconnected, even with `force=true` (user-initiated reconnect)
- After the first connection + disconnect, the `_connection` object
persists with the matching device ID but `disconnected` status
- Line 237 (`if (_connection?.device.id == deviceId) return null`)
blocks ALL subsequent reconnection attempts regardless of `force` — the
`force` flag was never checked on that path
- This made user-initiated reconnect taps silently do nothing on iOS
(Android was unaffected because `_connection` gets fully cleaned up on
disconnect)
- Fix: add `!force &&` so `force=true` calls fall through to
`_connectToDevice`

## Test plan
- [x] iOS: disconnect device manually, tap to reconnect — should
reconnect
- [x] iOS: device goes out of range and back — native auto-reconnect
still works (force=false path unchanged)
- [x] Android: no behavioral change expected

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant