Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Greptile Summary

This PR replaces the blocking ServiceRestartDialog (with its effectively-dead 15-second cancel button) with a non-modal inline ServiceRestartBanner composable across SettingsScreen, IdentityScreen, and DiscoveredInterfacesScreen. It also extracts the banner from duplicated inline code in DiscoveredInterfacesScreen into a shared component, and wires up an onServiceReady callback in InterfaceConfigManager.applyInterfaceChanges() so ViewModels can clear isRestarting right after Reticulum init (Step 9) rather than waiting for post-init bookkeeping.

Key changes:

• New ServiceRestartBanner composable (extracted + localised string with proper Unicode ellipsis)
• applyInterfaceChanges(onServiceReady: (() -> Unit)? = null) — callback fires after Step 9, before Steps 10–12
• SettingsViewModel and DiscoveredInterfacesViewModel correctly handle early failures: they use .getOrThrow() so their outer catch blocks reset isRestarting = false when the restart fails before Step 9
• DebugViewModel.restartService() does not call .getOrThrow() or otherwise check the Result. Because applyInterfaceChanges uses runCatching and never throws, a failure before Step 9 silently returns Result.failure, onServiceReady is never invoked, the catch block is never reached, and _isRestarting is permanently stuck at true — the IdentityScreen banner never dismisses. The fix is one line: check result.isFailure and reset the flag, mirroring the SettingsViewModel pattern.
• cancelRestart() removed from both SettingsViewModel and DebugViewModel — correct, as the banner no longer has a cancel affordance
• Tests updated throughout with mockApplyInterfaceChangesSuccess() helper and applyInterfaceChanges(any()) matchers

Confidence Score: 3/5

• Mostly safe to merge, but DebugViewModel has a concrete isRestarting leak that would leave the IdentityScreen banner stuck on any early-failure restart.
• 12 of 13 changed files are clean and well-tested. The single bug in DebugViewModel.restartService() is a one-line fix — the result of applyInterfaceChanges is discarded so the failure path never clears _isRestarting. This breaks the primary user path for IdentityScreen restarts (banner never dismisses on failure), warranting a 3 rather than a 4.
• app/src/main/java/com/lxmf/messenger/viewmodel/DebugViewModel.kt — restartService() does not handle Result.failure from applyInterfaceChanges, leaving _isRestarting stuck on early failures.

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant VM as ViewModel
    participant ICM as InterfaceConfigManager
    participant UI as Screen (Banner)

    VM->>UI: isRestarting = true → banner appears
    VM->>ICM: applyInterfaceChanges(onServiceReady)
    ICM-->>ICM: Steps 1–8 (shutdown, restart, bind…)
    ICM-->>ICM: Step 9: Reticulum init ✓
    ICM-->>VM: onServiceReady() callback
    VM->>UI: isRestarting = false → banner disappears
    ICM-->>ICM: Steps 10–12 (identity restore, manager restart)
    ICM-->>VM: Result.success(Unit)

    note over VM,ICM: On early failure (Steps 1–9 fail)
    ICM-->>VM: Result.failure (no throw) [SettingsVM/.getOrThrow() re-throws → catch resets flag ✓]
    note over VM: DebugViewModel: result discarded,<br/>catch never fires → isRestarting stuck true ✗

This is a comment left during a code review.
Path: app/src/main/java/com/lxmf/messenger/viewmodel/DebugViewModel.kt
Line: 581-587

Comment:
**`isRestarting` stuck `true` on early-failure path**

`applyInterfaceChanges` wraps all failures in `runCatching` and returns `Result.failure` — it never throws. This means the outer `catch` block on line 585 only fires for exceptions raised by other code in the `try` block (practically nothing), not for a failed restart.

- **Happy path**: Step 9 succeeds → `onServiceReady` fires → `_isRestarting.value = false` ✓  
- **Late failure** (Step 10–12 fail): `onServiceReady` already fired → `_isRestarting` already `false` ✓  
- **Early failure** (Steps 1–9 fail, e.g. timeout, service shutdown error): `onServiceReady` is **never invoked**, `applyInterfaceChanges` returns `Result.failure` silently, `catch` block never runs → `_isRestarting` stays `true` forever ✗

In contrast, `SettingsViewModel.restartService()` avoids this by calling `.getOrThrow()` which re-throws the failure, allowing its `catch` block to reset the flag.

Fix: either ignore-and-reset, or mirror the `SettingsViewModel` pattern:

```suggestion
                    interfaceConfigManager.applyInterfaceChanges(
                        onServiceReady = { _isRestarting.value = false },
                    ).also { result ->
                        if (result.isFailure) _isRestarting.value = false
                    }
                    Log.i(TAG, "Service restart completed successfully")
                } catch (e: Exception) {
                    Log.e(TAG, "Error restarting service", e)
                    _isRestarting.value = false
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (3): Last reviewed commit: "fix: update test to match unicode ellips..." | Re-trigger Greptile}