From 01fc5f3ac6da1a8ef2fdac6e43360d06de7341be Mon Sep 17 00:00:00 2001 From: nrobi144 Date: Thu, 2 Apr 2026 12:13:00 +0300 Subject: [PATCH] fix(tor): remove broken runtime reconnect, add evictConnections Remove the LaunchedEffect/scope.launch reconnect code that didn't work (relay subscriptions lost after disconnect+connect cycle). Tor toggle will use app rebuild via key() instead (next commit). Co-Authored-By: Claude Opus 4.6 (1M context) --- .../vitorpamplona/amethyst/desktop/Main.kt | 20 -- .../desktop/network/DesktopHttpClient.kt | 7 +- ...6-04-02-feat-tor-toggle-restart-ux-plan.md | 183 ++++++++++++++++++ 3 files changed, 189 insertions(+), 21 deletions(-) create mode 100644 docs/plans/2026-04-02-feat-tor-toggle-restart-ux-plan.md diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt index 14ff65ba81..528a9d9cba 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/Main.kt @@ -500,26 +500,6 @@ fun App( } val relayManager = remember(httpClient) { DesktopRelayConnectionManager(httpClient) } - // Reconnect relays on Tor state transitions (ON→proxy, OFF→direct) - LaunchedEffect(torManager, relayManager) { - var previousStatus: com.vitorpamplona.amethyst.commons.tor.TorServiceStatus? = null - torManager.status.collect { status -> - val changed = previousStatus != null && previousStatus != status - // Reconnect on meaningful transitions: Active (proxy ready) or Off (switch to direct) - val isTransition = - changed && - ( - status is com.vitorpamplona.amethyst.commons.tor.TorServiceStatus.Active || - status is com.vitorpamplona.amethyst.commons.tor.TorServiceStatus.Off - ) - if (isTransition) { - kotlinx.coroutines.delay(500) // Brief delay for client state to propagate - relayManager.client.reconnect(onlyIfChanged = false, ignoreRetryDelays = true) - } - previousStatus = status - } - } - // Subscriptions coordinator — uses default relay URLs for metadata indexing. // Feed subscriptions (inside MainContent) drive actual relay pool connections. val subscriptionsCoordinator = diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/network/DesktopHttpClient.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/network/DesktopHttpClient.kt index a43aa10830..a63091f7a9 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/network/DesktopHttpClient.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/network/DesktopHttpClient.kt @@ -56,7 +56,12 @@ class DesktopHttpClient( /** Returns true if user expects Tor routing (INTERNAL or EXTERNAL mode). */ fun isTorExpected(): Boolean = torTypeProvider() != TorType.OFF - private val sharedConnectionPool = ConnectionPool() + val sharedConnectionPool = ConnectionPool() + + /** Evict all idle connections — call on Tor state change to kill stale direct/proxy connections. */ + fun evictConnections() { + sharedConnectionPool.evictAll() + } private val directClient: OkHttpClient by lazy { OkHttpClient diff --git a/docs/plans/2026-04-02-feat-tor-toggle-restart-ux-plan.md b/docs/plans/2026-04-02-feat-tor-toggle-restart-ux-plan.md new file mode 100644 index 0000000000..f4e6a0d694 --- /dev/null +++ b/docs/plans/2026-04-02-feat-tor-toggle-restart-ux-plan.md @@ -0,0 +1,183 @@ +--- +title: "feat: Tor toggle restart UX — confirmation dialog + app rebuild" +type: feat +status: active +date: 2026-04-02 +deepened: 2026-04-02 +--- + +# feat: Tor toggle restart UX + +## Enhancement Summary + +**Deepened on:** 2026-04-02 +**Agents:** Cleanup verifier, Simplicity reviewer, Compose Desktop edge cases + +### Key Improvements +1. **No new files** — inline AlertDialog, merge restart into `onSettingsChanged` +2. **DisposableEffect for TorManager** — critical: stop Tor daemon on unmount to prevent process leak +3. **Restart on ALL settings changes** — simpler than mode-only distinction, and toggle changes don't propagate at runtime either +4. **key() confirmed safe** — well-understood Compose pattern, scopes cancel, remember blocks reset + +--- + +## Overview + +When user changes any Tor setting, show confirmation dialog. On confirm: save prefs → `appRestartKey++` → Compose unmounts App tree → remounts fresh. Window stays open, user stays logged in. + +## Problem Statement + +Toggling Tor at runtime breaks relay subscriptions. TCP connections switch but `DesktopRelaySubscriptionsCoordinator` doesn't re-subscribe. Hot-toggle requires deep quartz changes. Simpler: rebuild app state via `key()`. + +## Implementation + +### Phase 1: Add `appRestartKey` + TorManager DisposableEffect + +**File: `Main.kt`** + +At Window level (~line 187): +```kotlin +var appRestartKey by remember { mutableStateOf(0) } +``` + +Wrap `App(...)` call (~line 413): +```kotlin +key(appRestartKey) { + App(...) +} +``` + +**CRITICAL: Add DisposableEffect for TorManager inside App:** +```kotlin +DisposableEffect(torManager) { + onDispose { + torManager.stopSync() + activeTorManager = null + } +} +``` +Without this, old Tor daemon keeps running → two Tor processes → port conflicts. + +**Merge restart into onSettingsChanged** in CompositionLocalProvider: +```kotlin +TorState( + status = currentTorStatus, + settings = torSettings, + onSettingsChanged = { newSettings -> + torSettings = newSettings + DesktopTorPreferences.save(newSettings) + torTypeFlow.value = newSettings.torType + externalPortFlow.value = newSettings.externalSocksPort + // Restart app to apply Tor changes + appRestartKey++ + }, +) +``` + +No separate `onRestartRequired` callback needed. + +### Phase 2: Add confirmation dialog to TorSettingsSection + +**File: `TorSettingsSection.kt`** + +Before applying changes, show inline confirmation: +```kotlin +var pendingSettings by remember { mutableStateOf(null) } + +// Mode selector onClick: +onClick = { + pendingSettings = currentSettings.copy(torType = torType) +} + +// Confirmation dialog: +pendingSettings?.let { settings -> + AlertDialog( + onDismissRequest = { pendingSettings = null }, + title = { Text("Restart Required") }, + text = { Text("Changing Tor settings requires restarting. Your session will be briefly interrupted.") }, + confirmButton = { + TextButton(onClick = { + onSettingsChanged(settings) + pendingSettings = null + }) { Text("Restart") } + }, + dismissButton = { + TextButton(onClick = { pendingSettings = null }) { Text("Cancel") } + }, + ) +} +``` + +### Phase 3: Add confirmation to TorSettingsDialog Save + +**File: `TorSettingsDialog.kt`** + +Same pattern — Save button shows confirmation before calling `onSettingsChanged`: +```kotlin +var showRestartConfirm by remember { mutableStateOf(false) } + +// Save button: +TextButton(onClick = { showRestartConfirm = true }) { Text("Save") } + +if (showRestartConfirm) { + AlertDialog( + onDismissRequest = { showRestartConfirm = false }, + title = { Text("Restart Required") }, + text = { Text("Tor changes require restarting. Proceed?") }, + confirmButton = { + TextButton(onClick = { + onSettingsChanged(editSettings) + onDismiss() + }) { Text("Restart") } + }, + dismissButton = { + TextButton(onClick = { showRestartConfirm = false }) { Text("Cancel") } + }, + ) +} +``` + +### Phase 4: Cleanup dead code + +Remove from `Main.kt`: +- Any reconnect-related code (torReconnectJob, evictConnections calls) +- Debug logging (TorReconnect log lines) + +Remove `evictConnections()` from `DesktopHttpClient.kt` (YAGNI — add back when needed). + +## What Survives the key() Restart + +| Component | Survives? | Why | +|-----------|-----------|-----| +| AccountManager | Yes | Outside App, at Window level | +| DeckState (columns) | Yes | Outside App, at Window level | +| Window position/size | Yes | windowState is outside App | +| Login session | Yes | AccountManager persists | +| Tor settings | Yes | Saved to java.util.prefs before restart | +| CoroutineScope | No — recreated | DisposableEffect cancels old | +| TorManager | No — recreated | DisposableEffect stops daemon | +| RelayManager | No — recreated | DisposableEffect disconnects | +| Subscriptions | No — recreated | DisposableEffect clears | +| Coil ImageLoader | Yes | Process-global singleton, not in Compose tree | + +## Acceptance Criteria + +- [ ] Any Tor setting change shows "Restart Required" dialog +- [ ] Cancel reverts — no settings change, no restart +- [ ] Confirm saves prefs + rebuilds app (window stays open) +- [ ] User stays logged in after rebuild +- [ ] Column layout preserved after rebuild +- [ ] Relays connected with correct mode after rebuild +- [ ] Feed loads after rebuild +- [ ] Old Tor daemon stopped before new one starts (no process leak) +- [ ] Shield icon correct after rebuild + +## Files Changed + +| File | Change | +|------|--------| +| `Main.kt` | Add `appRestartKey`, `key()` wrapper, `DisposableEffect` for TorManager, restart in `onSettingsChanged` | +| `TorSettingsSection.kt` | Confirmation dialog before applying mode change | +| `TorSettingsDialog.kt` | Confirmation dialog before Save | + +**3 files changed, 0 new files, ~40 lines added.**