From 90c1e8dc5f3a04a4df8c34927ebd33b0396bf03a Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:27:36 +0300 Subject: [PATCH] =?UTF-8?q?fix:=20tab=20reloads=20actually=20reload=20?= =?UTF-8?q?=E2=80=94=20kanban=20switch=20spam,=20plugins/skills=20refresh,?= =?UTF-8?q?=20ws=20mint=20(#897)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ws: ticket-mint response body was read on the calling (main) thread — the socket read threw NetworkOnMainThreadException, the mint failed, and the kanban events stream never connected (board stuck Offline). execute() + body read now run entirely on Dispatchers.IO; same fix in DashboardSessionTokenRefresher.fetch (loopback mint path). - kanban: re-selecting the already-active board reloads data and reconnects events WITHOUT the redundant POST /boards/{id}/switch that refresh fired on every tap. - plugins: top-bar Refresh + pull-to-refresh wired through HermesScaffold (previously no reload control at all). - skills: Refresh icon now does a plain GET reload; the heavy hub-update POST moved to the cloud icon so refresh is no longer disguised as update. - test: mock the 3-arg Log.w overload (catch now logs the full stack). Co-authored-by: M57 --- .../remote/DashboardSessionTokenRefresher.kt | 39 +++-- .../hermescontrol/data/ws/HermesWsClient.kt | 38 +++-- .../ui/kanban/KanbanViewModel.kt | 9 ++ .../hermescontrol/ui/plugins/PluginsScreen.kt | 139 +++++++++--------- .../hermescontrol/ui/skills/SkillsScreen.kt | 11 +- .../data/ws/HermesWsClientTest.kt | 1 + 6 files changed, 131 insertions(+), 106 deletions(-) diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/DashboardSessionTokenRefresher.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/DashboardSessionTokenRefresher.kt index 7c8f6b39..705b2195 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/DashboardSessionTokenRefresher.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/DashboardSessionTokenRefresher.kt @@ -1,6 +1,7 @@ package com.m57.hermescontrol.data.remote import com.m57.hermescontrol.data.local.AuthManager +import kotlinx.coroutines.Dispatchers import okhttp3.OkHttpClient import okhttp3.Request import okio.Buffer @@ -31,22 +32,28 @@ internal object DashboardSessionTokenRefresher { .url(baseUrl) .get() .build() - client.newCall(request).execute().use { response -> - if (!response.isSuccessful) return null - // Cap the read — the SPA HTML can be large, and we only need the - // injected token near the top of the document. Reading the whole - // body into a String risks OOM on low-end devices. - val html = - response.body.source().use { source -> - val buffer = Buffer() - source.read(buffer, MAX_BODY_BYTES) - buffer.readUtf8() - } - tokenPattern - .find(html) - ?.groupValues - ?.getOrNull(1) - ?.takeIf { it.isNotBlank() } + // execute() + body read on Dispatchers.IO: the body read touches + // the socket on the CALLING thread otherwise — when that is main + // (loopback ticket mint from the kanban events connect), it throws + // NetworkOnMainThreadException (same bug class as requestWsTicket). + kotlinx.coroutines.runBlocking(Dispatchers.IO) { + client.newCall(request).execute().use { response -> + if (!response.isSuccessful) return@runBlocking null + // Cap the read — the SPA HTML can be large, and we only need the + // injected token near the top of the document. Reading the whole + // body into a String risks OOM on low-end devices. + val html = + response.body.source().use { source -> + val buffer = Buffer() + source.read(buffer, MAX_BODY_BYTES) + buffer.readUtf8() + } + tokenPattern + .find(html) + ?.groupValues + ?.getOrNull(1) + ?.takeIf { it.isNotBlank() } + } } } catch (_: Exception) { null diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt index e9742e19..ea368a87 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt @@ -450,28 +450,34 @@ object HermesWsClient { .post("{}".toRequestBody()) .build() - // Run network call on Dispatchers.IO to avoid NetworkOnMainThreadException - val response = + // Run the ENTIRE call on Dispatchers.IO — execute() already hops, + // but ResponseBody.string() reads the socket on the CALLING + // thread. When the caller is main (kanban events connect), that + // read throws NetworkOnMainThreadException and the mint fails. + val (code, body) = kotlinx.coroutines.runBlocking(Dispatchers.IO) { - client.newCall(request).execute() + client.newCall(request).execute().use { resp -> + resp.code to resp.body?.string() + } } - response.use { - if (response.isSuccessful) { - val body = response.body.string() - val ticketMatch = Regex("""\"ticket\":\"([^\"]+)\"""").find(body) - val ticket = ticketMatch?.groupValues?.getOrNull(1) - if (!ticket.isNullOrBlank()) { - return TicketRequestResult(ticket, null) - } - Log.w(TAG, "WS ticket mint failed: response body did not contain ticket") - } else { - Log.w(TAG, "WS ticket mint failed: HTTP ${response.code}") - return TicketRequestResult(null, response.code) + if (body != null && code in 200..299) { + val ticketMatch = Regex("""\"ticket\":\"([^\"]+)\"""").find(body) + val ticket = ticketMatch?.groupValues?.getOrNull(1) + if (!ticket.isNullOrBlank()) { + return TicketRequestResult(ticket, null) } + Log.w(TAG, "WS ticket mint failed: response body did not contain ticket") + } else { + Log.w(TAG, "WS ticket mint failed: HTTP $code") + return TicketRequestResult(null, code) } } catch (e: Exception) { - Log.w(TAG, "WS ticket mint failed: ${e.javaClass.simpleName}") + // Include the stack: the exception class alone (e.g. + // NetworkOnMainThreadException) can't show WHICH caller on what + // thread tripped the mint (main-scope kanban events connect vs + // the WS reconnect path). + Log.w(TAG, "WS ticket mint failed: ${e.javaClass.simpleName}", e) return TicketRequestResult(null, null, exception = true) } return TicketRequestResult(null, null) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/kanban/KanbanViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/kanban/KanbanViewModel.kt index 1c2dbac4..051cca9b 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/kanban/KanbanViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/kanban/KanbanViewModel.kt @@ -121,6 +121,15 @@ class KanbanViewModel( } fun selectBoard(board: KanbanBoard) { + val current = _uiState.value.selectedBoard + if (current?.id == board.id) { + // Already-active board: reload its data and (re)connect the events + // stream WITHOUT the /switch round-trip — refresh and createTask + // re-select the same board and must not POST a redundant switch. + viewModelScope.launch { loadBoardIntoState(board) } + connectEvents(board) + return + } _uiState.update { it.copy(selectedBoard = board, isLoading = true, errorMessage = null) } viewModelScope.launch { val switchResult = diff --git a/app/src/main/java/com/m57/hermescontrol/ui/plugins/PluginsScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/plugins/PluginsScreen.kt index b166a137..9cd816f1 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/plugins/PluginsScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/plugins/PluginsScreen.kt @@ -32,7 +32,6 @@ import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.TextButton -import androidx.compose.material3.pulltorefresh.PullToRefreshBox import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue @@ -121,86 +120,82 @@ fun PluginsScreen( HermesScaffold( title = { Text(stringResource(R.string.screen_plugins)) }, navigationIcon = onOpenDrawer?.let { NavIcon.Menu(it) }, + isRefreshing = state.isLoading, + onRefresh = { viewModel.loadPlugins() }, ) { paddingValues -> - PullToRefreshBox( - isRefreshing = state.isLoading, - onRefresh = { viewModel.loadPlugins() }, - modifier = Modifier.fillMaxSize(), - ) { - when { - state.isLoading && state.plugins.isEmpty() -> { - SkeletonListState(modifier = Modifier.padding(paddingValues)) - } + when { + state.isLoading && state.plugins.isEmpty() -> { + SkeletonListState(modifier = Modifier.padding(paddingValues)) + } - state.errorMessage != null -> { - ErrorState( - message = state.errorMessage ?: "", - onRetry = { viewModel.loadPlugins() }, - modifier = Modifier.padding(paddingValues), - ) - } + state.errorMessage != null -> { + ErrorState( + message = state.errorMessage ?: "", + onRetry = { viewModel.loadPlugins() }, + modifier = Modifier.padding(paddingValues), + ) + } - state.plugins.isEmpty() && state.orphanPlugins.isEmpty() -> { - EmptyState( - title = stringResource(R.string.plugins_empty_title), - subtitle = stringResource(R.string.plugins_empty_desc), - onAction = { viewModel.loadPlugins() }, - actionLabel = stringResource(R.string.content_desc_refresh), - modifier = Modifier.padding(paddingValues), - ) - } + state.plugins.isEmpty() && state.orphanPlugins.isEmpty() -> { + EmptyState( + title = stringResource(R.string.plugins_empty_title), + subtitle = stringResource(R.string.plugins_empty_desc), + onAction = { viewModel.loadPlugins() }, + actionLabel = stringResource(R.string.content_desc_refresh), + modifier = Modifier.padding(paddingValues), + ) + } - else -> { - LazyColumn( - modifier = Modifier.fillMaxSize(), - contentPadding = listContentPadding, - verticalArrangement = listItemSpacing, - ) { - // Provider selection section - if (state.memoryOptions.isNotEmpty() || state.contextOptions.isNotEmpty()) { - item(key = "providers") { - ProviderSelectionSection(state, viewModel) - } + else -> { + LazyColumn( + modifier = Modifier.fillMaxSize(), + contentPadding = listContentPadding, + verticalArrangement = listItemSpacing, + ) { + // Provider selection section + if (state.memoryOptions.isNotEmpty() || state.contextOptions.isNotEmpty()) { + item(key = "providers") { + ProviderSelectionSection(state, viewModel) } + } - // Install section - item(key = "install") { - InstallSection(state, viewModel) - } + // Install section + item(key = "install") { + InstallSection(state, viewModel) + } - // Search bar - item(key = "search") { - SearchBar( - query = query, - onQueryChange = { query = it }, - placeholder = "Search plugins...", + // Search bar + item(key = "search") { + SearchBar( + query = query, + onQueryChange = { query = it }, + placeholder = "Search plugins...", + ) + } + + // Plugin list + items(filteredPlugins, key = { it.name }) { plugin -> + PluginCard( + plugin = plugin, + state = state, + viewModel = viewModel, + onClick = { showDetail = plugin }, + ) + } + + // Orphan dashboard plugins section + if (state.orphanPlugins.isNotEmpty()) { + item(key = "orphan-header") { + Spacer(modifier = Modifier.height(8.dp)) + Text( + text = stringResource(R.string.plugins_orphan_heading), + style = MaterialTheme.typography.titleSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(vertical = 8.dp), ) } - - // Plugin list - items(filteredPlugins, key = { it.name }) { plugin -> - PluginCard( - plugin = plugin, - state = state, - viewModel = viewModel, - onClick = { showDetail = plugin }, - ) - } - - // Orphan dashboard plugins section - if (state.orphanPlugins.isNotEmpty()) { - item(key = "orphan-header") { - Spacer(modifier = Modifier.height(8.dp)) - Text( - text = stringResource(R.string.plugins_orphan_heading), - style = MaterialTheme.typography.titleSmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(vertical = 8.dp), - ) - } - items(state.orphanPlugins, key = { "orphan-${it.name}" }) { plugin -> - OrphanPluginCard(plugin = plugin, onClick = { showDetail = plugin }) - } + items(state.orphanPlugins, key = { "orphan-${it.name}" }) { plugin -> + OrphanPluginCard(plugin = plugin, onClick = { showDetail = plugin }) } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillsScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillsScreen.kt index a9517af8..4718dc5a 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillsScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/skills/SkillsScreen.kt @@ -28,7 +28,6 @@ import androidx.compose.material.icons.filled.CloudDownload import androidx.compose.material.icons.filled.CloudOff import androidx.compose.material.icons.filled.Edit import androidx.compose.material.icons.filled.Extension -import androidx.compose.material.icons.filled.Refresh import androidx.compose.material.icons.filled.Save import androidx.compose.material.icons.filled.Search import androidx.compose.material.icons.filled.Shield @@ -113,11 +112,19 @@ fun SkillsScreen( modifier = modifier, title = { Text(stringResource(R.string.skills_screen_title)) }, navigationIcon = NavIcon.Menu(onOpen = onOpenDrawer), + isRefreshing = state.isLoading, + onRefresh = { + if (state.viewMode == SkillsViewMode.INSTALLED) { + viewModel.loadSkills() + } else { + viewModel.loadHubSources() + } + }, actions = { if (state.viewMode == SkillsViewMode.INSTALLED) { IconButton(onClick = { viewModel.updateSkillsFromHub() }) { Icon( - imageVector = Icons.Filled.Refresh, + imageVector = Icons.Filled.CloudDownload, contentDescription = stringResource(R.string.content_desc_update_skills_hub), ) } diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt index 430e64d8..40ca4762 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt @@ -49,6 +49,7 @@ class HermesWsClientTest { every { Log.d(any(), any()) } returns 0 every { Log.i(any(), any()) } returns 0 every { Log.w(any(), any()) } returns 0 + every { Log.w(any(), any(), any()) } returns 0 every { Log.e(any(), any(), any()) } returns 0 mockWebServer = MockWebServer()